From 694dac75caf400ccd6a2d66d4328e0f0e2994edf Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Thu, 16 Nov 2017 14:18:11 -0500 Subject: [PATCH] Define resource/output definition sections with constants It was unclear what the valid arguments to Template.get_section_name() were (especially since the function is mis-named for what it actually does in HOT). Define the arguments as constants and don't pass string literals any more. Be consistent in how we define paths, standardising on the method in Resource.validate_template(). Change-Id: Ifd073d9889ff60502f78aaa54532cec2b7814d93 --- heat/engine/output.py | 13 +++++++++++-- .../resources/openstack/sahara/job_binary.py | 5 +++-- .../resources/openstack/sahara/templates.py | 5 +++-- heat/engine/rsrc_defn.py | 19 +++++++++++++++---- heat/engine/stack.py | 16 +++++++++------- heat/engine/template.py | 11 ++++++++++- 6 files changed, 51 insertions(+), 18 deletions(-) diff --git a/heat/engine/output.py b/heat/engine/output.py index 73c41e62fa..1c12232cea 100644 --- a/heat/engine/output.py +++ b/heat/engine/output.py @@ -19,6 +19,15 @@ from heat.common import exception from heat.engine import function +# Field names that can be passed to Template.get_section_name() in order to +# determine the appropriate name for a particular template format. +FIELDS = ( + VALUE, DESCRIPTION, +) = ( + 'Value', 'Description', +) + + class OutputDefinition(object): """A definition of a stack output, independent of any template format.""" @@ -30,9 +39,9 @@ class OutputDefinition(object): self._deps = None self._all_dep_attrs = None - def validate(self, path=''): + def validate(self): """Validate the output value without resolving it.""" - function.validate(self._value, path) + function.validate(self._value, VALUE) def required_resource_names(self): if self._deps is None: diff --git a/heat/engine/resources/openstack/sahara/job_binary.py b/heat/engine/resources/openstack/sahara/job_binary.py index f59cbf8056..85cf705f2c 100644 --- a/heat/engine/resources/openstack/sahara/job_binary.py +++ b/heat/engine/resources/openstack/sahara/job_binary.py @@ -18,6 +18,7 @@ from heat.common import exception from heat.common.i18n import _ from heat.engine import properties from heat.engine import resource +from heat.engine import rsrc_defn from heat.engine import support @@ -105,8 +106,8 @@ class JobBinary(resource.Resource): and uuidutils.is_uuid_like(url[len("internal-db://"):]))): msg = _("%s is not a valid job location.") % url raise exception.StackValidationFailed( - path=[self.stack.t.get_section_name('resources'), self.name, - self.stack.t.get_section_name('properties')], + path=[self.stack.t.RESOURCES, self.name, + self.stack.t.get_section_name(rsrc_defn.PROPERTIES)], message=msg) def handle_create(self): diff --git a/heat/engine/resources/openstack/sahara/templates.py b/heat/engine/resources/openstack/sahara/templates.py index 78a9eb1c21..eb4bc738f4 100644 --- a/heat/engine/resources/openstack/sahara/templates.py +++ b/heat/engine/resources/openstack/sahara/templates.py @@ -24,6 +24,7 @@ from heat.common.i18n import _ from heat.engine import constraints from heat.engine import properties from heat.engine import resource +from heat.engine import rsrc_defn from heat.engine import support from heat.engine import translation @@ -346,9 +347,9 @@ class SaharaNodeGroupTemplate(resource.Resource): 'unsupported': ', '.join(unsupported_processes), 'allowed': ', '.join(allowed_processes)}) raise exception.StackValidationFailed( - path=[self.stack.t.get_section_name('resources'), + path=[self.stack.t.RESOURCES, self.name, - self.stack.t.get_section_name('properties')], + self.stack.t.get_section_name(rsrc_defn.PROPERTIES)], message=msg) def parse_live_resource_data(self, resource_properties, resource_data): diff --git a/heat/engine/rsrc_defn.py b/heat/engine/rsrc_defn.py index c5537ce74e..63c2cf8496 100644 --- a/heat/engine/rsrc_defn.py +++ b/heat/engine/rsrc_defn.py @@ -26,6 +26,17 @@ from heat.engine import properties __all__ = ['ResourceDefinition'] +# Field names that can be passed to Template.get_section_name() in order to +# determine the appropriate name for a particular template format. +FIELDS = ( + TYPE, PROPERTIES, METADATA, DELETION_POLICY, UPDATE_POLICY, + DEPENDS_ON, DESCRIPTION, EXTERNAL_ID, +) = ( + 'Type', 'Properties', 'Metadata', 'DeletionPolicy', 'UpdatePolicy', + 'DependsOn', 'Description', 'external_id', +) + + @repr_wrapper class ResourceDefinition(object): """A definition of a resource, independent of any template format.""" @@ -230,9 +241,9 @@ class ResourceDefinition(object): return '.'.join([self.name, section]) prop_deps = function.dependencies(self._properties, - path('Properties')) + path(PROPERTIES)) metadata_deps = function.dependencies(self._metadata, - path('Metadata')) + path(METADATA)) implicit_depends = six.moves.map(lambda rp: rp.name, itertools.chain(prop_deps, metadata_deps)) @@ -282,7 +293,7 @@ class ResourceDefinition(object): """ props = properties.Properties(schema, self._properties or {}, function.resolve, context=context, - section='Properties') + section=PROPERTIES) props.update_translation(self._rules, self._client_resolve) return props @@ -301,7 +312,7 @@ class ResourceDefinition(object): """ props = properties.Properties(schema, self._update_policy or {}, function.resolve, context=context, - section='UpdatePolicy') + section=UPDATE_POLICY) props.update_translation(self._rules, self._client_resolve) return props diff --git a/heat/engine/stack.py b/heat/engine/stack.py index 16e5d2e558..6aa56b0dfb 100644 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -904,13 +904,15 @@ class Stack(collections.Mapping): for op_name, output in six.iteritems(self.outputs): try: - path = '.'.join([self.t.OUTPUTS, op_name, - self.t.OUTPUT_VALUE]) - output.validate(path) - except exception.StackValidationFailed: - raise - except AssertionError: - raise + output.validate() + except exception.StackValidationFailed as ex: + path = [self.t.OUTPUTS, op_name, + self.t.get_section_name(ex.path[0])] + path.extend(ex.path[1:]) + raise exception.StackValidationFailed( + error=ex.error, + path=path, + message=ex.error_message) def requires_deferred_auth(self): """Determine whether to perform API requests with deferred auth. diff --git a/heat/engine/template.py b/heat/engine/template.py index cb5551c4e9..8493e4069e 100644 --- a/heat/engine/template.py +++ b/heat/engine/template.py @@ -213,7 +213,16 @@ class Template(collections.Mapping): @abc.abstractmethod def get_section_name(self, section): - """Return a correct section name.""" + """Get the name of a field within a resource or output definition. + + Return the name of the given field (specified by the constants given + in heat.engine.rsrc_defn and heat.engine.output) in the template + format. This is used in error reporting to help users find the + location of errors in the template. + + Note that 'section' here does not refer to a top-level section of the + template (like parameters, resources, &c.) as it does everywhere else. + """ pass @abc.abstractmethod