openstack
/
heat
Code Issues Proposed changes

Fix HOT inconsistencies in resource sections 

This patch fixes inconsistencies between keywords in resource
definitions in a HOT template as documented in the HOT specification
and the way they are processed by the code.

So far it was possible to use CFN like syntax or HOT syntax inside
resource definitions (e.g. both 'properties' and 'Properties' were
accepted) since the HOT processing code did tolerant processing.
Consequently, users had been able to produce CFN/HOT mixed templates.

This patch introduces strict enforcement of allowed keywords to
make HOT templates consistent with the spec by providing concrete
validation errors in case of invalid keywords.

This patch also adds a DB migration step to update existing templates
in the Heat database with potentially legacy keywords to comply with
the new strict syntax checking rules.

Change-Id: I44309acda86a930030cb70493f409e3ea4740130
Closes-Bug: #1274288
Closes-Bug: #1271008
This commit is contained in:
Thomas Spatzier 2014-01-27 19:43:59 +01:00
parent 22fd357a5e
commit 18bd016388
6 changed files with 389 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
doc/source/template_guide/hot_spec.rst
View File

@ -398,7 +398,11 @@ to the syntax below.
type: <resource type>
properties:
<property name>: <property value>
# more resource specific metadata
metadata:
<resource specific metadata>
depends_on: <resource ID or list of ID>
update_poliy: <update policy>
deletion_policy: <deletion policy>
resource ID
A resource block is headed by the resource ID, which must be unique within
@ -406,12 +410,28 @@ resource ID
type
This attribute specifies the type of resource, such as OS::Nova::Server.
properties
This section contains a list of resource specific properties. The property
value can be provided in place, or can be provided via a function
(see :ref:`hot_spec_intrinsic_functions`).
This *optional* section contains a list of resource specific properties.
The property value can be provided in place, or can be provided via a
function (see :ref:`hot_spec_intrinsic_functions`).
metadata
This *optional* section contains resource type specific metadata.
depends_on
This *optional* attribute allows for specifying dependencies of the current
resource on one or more other resources. Please refer to section
:ref:`hot_spec_resources_dependencies` for details.
update_policy:
This *optional* attribute allows for specifying an update policy for the
resource in the form of a nested dictionary (name-value pairs). Whether
update policies are supported and what the exact semantics are depends on
the type of the current resource.
deletion_policy:
This *optional* attribute allows for specifying a deletion policy for the
resource (one of the values Delete, Retain or Snapshot). Which type of
deletion policy is supported depends on the type of the current resource.
Depending on the type of resource, the resource block might include more
resource specific metadata. Basically all resource types that can be used in
resource specific data. Basically all resource types that can be used in
CFN templates can also be used in HOT templates, adapted to the YAML structure
as outlined above.
Below is an example of a simple compute resource definition with some fixed
@ -427,6 +447,45 @@ property values.
image: F18-x86_64-cfntools
.. _hot_spec_resources_dependencies:
Resource Dependencies
~~~~~~~~~~~~~~~~~~~~~
By means of the *depends_on* attribute within a resource section it is possible
to define a dependency between a resource and one or more other resources. If
a resource depends on just one other resource, the ID of the other resource is
specified as value of the *depends_on* attribute as shown in the following
example.
::
resources:
server1:
type: OS::Nova::Server
depends_on: server2
server2:
type: OS::Nova::Server
If a resource depends on more than one other resource, the value of the
*depends_on* attribute is specified as a list of resource IDs as shown in the
following example:
::
resources:
server1:
type: OS::Nova::Server
depends_on: [ server2, server3 ]
server2:
type: OS::Nova::Server
server3:
type: OS::Nova::Server
.. _hot_spec_outputs:
---------------

71
heat/db/sqlalchemy/migrate_repo/versions/041_migrate_hot_template_resources.py Normal file
View File

@ -0,0 +1,71 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import copy
from migrate.versioning import util as migrate_util
from sqlalchemy.orm import sessionmaker
from heat.openstack.common.gettextutils import _
from heat.db.sqlalchemy import models
def upgrade(migrate_engine):
Session = sessionmaker(bind=migrate_engine)
session = Session()
raw_templates = session.query(models.RawTemplate).all()
CFN_TO_HOT_RESOURCE_ATTRS = {'Type': 'type',
'Properties': 'properties',
'Metadata': 'metadata',
'DependsOn': 'depends_on',
'DeletionPolicy': 'deletion_policy',
'UpdatePolicy': 'update_policy'}
CFN_TO_HOT_OUTPUT_ATTRS = {'Description': 'description',
'Value': 'value'}
def _translate(section, translate_map):
changed = False
for name, details in section.iteritems():
for old_key, new_key in translate_map.iteritems():
if old_key in details:
details[new_key] = details[old_key]
del details[old_key]
changed = True
return changed
for raw_template in raw_templates:
if 'heat_template_version' in raw_template.template:
changed = False
template = copy.deepcopy(raw_template.template)
resources = template.get('resources', {})
if _translate(resources, CFN_TO_HOT_RESOURCE_ATTRS):
changed = True
outputs = template.get('outputs', {})
if _translate(outputs, CFN_TO_HOT_OUTPUT_ATTRS):
changed = True
if changed:
raw_template.template = template
session.commit()
def downgrade(migrate_engine):
migrate_util.log.warning(_('This version cannot be downgraded because '
'it involves a data migration to the '
'raw_template table.'))

31
heat/engine/hot/template.py
View File

@ -44,8 +44,10 @@ class HOTemplate(template.Template):
def __getitem__(self, section):
""""Get the relevant section in the template."""
#first translate from CFN into HOT terminology if necessary
section = HOTemplate._translate(section,
self._CFN_TO_HOT_SECTIONS, section)
if section not in self.SECTIONS:
section = HOTemplate._translate(section, self._CFN_TO_HOT_SECTIONS,
_('"%s" is not a valid template '
'section'))
if section not in self.SECTIONS:
raise KeyError(_('"%s" is not a valid template section') % section)
@ -77,16 +79,23 @@ class HOTemplate(template.Template):
return the_section
@staticmethod
def _translate(value, mapping, default=None):
if value in mapping:
def _translate(value, mapping, err_msg=None):
try:
return mapping[value]
return default
except KeyError as ke:
if err_msg:
raise KeyError(err_msg % value)
else:
raise ke
def _translate_resources(self, resources):
"""Get the resources of the template translated into CFN format."""
HOT_TO_CFN_ATTRS = {'type': 'Type',
'properties': 'Properties'}
'properties': 'Properties',
'metadata': 'Metadata',
'depends_on': 'DependsOn',
'deletion_policy': 'DeletionPolicy',
'update_policy': 'UpdatePolicy'}
cfn_resources = {}
@ -94,7 +103,9 @@ class HOTemplate(template.Template):
cfn_resource = {}
for attr, attr_value in attrs.iteritems():
cfn_attr = self._translate(attr, HOT_TO_CFN_ATTRS, attr)
cfn_attr = self._translate(attr, HOT_TO_CFN_ATTRS,
_('"%s" is not a valid keyword '
'inside a resource definition'))
cfn_resource[cfn_attr] = attr_value
cfn_resources[resource_name] = cfn_resource
@ -112,7 +123,9 @@ class HOTemplate(template.Template):
cfn_output = {}
for attr, attr_value in attrs.iteritems():
cfn_attr = self._translate(attr, HOT_TO_CFN_ATTRS, attr)
cfn_attr = self._translate(attr, HOT_TO_CFN_ATTRS,
_('"%s" is not a valid keyword '
'inside an output definition'))
cfn_output[cfn_attr] = attr_value
cfn_outputs[output_name] = cfn_output

36
heat/engine/resources/autoscaling.py
View File

@ -292,6 +292,11 @@ class InstanceGroup(stack_resource.StackResource):
# resolve references within the context of this stack.
return self.stack.resolve_runtime_data(instance_definition)
def _get_instance_templates(self):
"""Get templates for resource instances."""
return [(instance.name, instance.t)
for instance in self.get_instances()]
def _create_template(self, num_instances, num_replace=0):
"""
Create a template to represent autoscaled instances.
@ -299,8 +304,7 @@ class InstanceGroup(stack_resource.StackResource):
Also see heat.scaling.template.resource_templates.
"""
instance_definition = self._get_instance_definition()
old_resources = [(instance.name, instance.t)
for instance in self.get_instances()]
old_resources = self._get_instance_templates()
templates = template.resource_templates(
old_resources, instance_definition, num_instances, num_replace)
return {"Resources": dict(templates)}
@ -870,6 +874,34 @@ class AutoScalingResourceGroup(AutoScalingGroup):
"""
pass
def _get_instance_templates(self):
"""
Get templates for resource instances in HOT format.
AutoScalingResourceGroup uses HOT as template format for scaled
resources. Templates for existing resources use CFN syntax and
have to be converted to HOT since those templates get used for
generating scaled resource templates.
"""
CFN_TO_HOT_ATTRS = {'Type': 'type',
'Properties': 'properties',
'Metadata': 'metadata',
'DependsOn': 'depends_on',
'DeletionPolicy': 'deletion_policy',
'UpdatePolicy': 'update_policy'}
def to_hot(template):
hot_template = {}
for attr, attr_value in template.iteritems():
hot_attr = CFN_TO_HOT_ATTRS.get(attr, attr)
hot_template[hot_attr] = attr_value
return hot_template
return [(instance.name, to_hot(instance.t))
for instance in self.get_instances()]
def _create_template(self, *args, **kwargs):
"""Use a HOT format for the template in the nested stack."""
tpl = super(AutoScalingResourceGroup, self)._create_template(

182
heat/tests/test_hot.py
View File

@ -83,7 +83,7 @@ class HOTemplateTest(HeatTestCase):
self.assertEqual({}, tmpl[tmpl.RESOURCES])
self.assertEqual({}, tmpl[tmpl.OUTPUTS])
def test_translate_resources(self):
def test_translate_resources_good(self):
"""Test translation of resources into internal engine format."""
hot_tpl = template_format.parse('''
@ -93,15 +93,157 @@ class HOTemplateTest(HeatTestCase):
type: AWS::EC2::Instance
properties:
property1: value1
metadata:
foo: bar
depends_on: dummy
deletion_policy: dummy
update_policy:
foo: bar
''')
expected = {'resource1': {'Type': 'AWS::EC2::Instance',
'Properties': {'property1': 'value1'}}}
'Properties': {'property1': 'value1'},
'Metadata': {'foo': 'bar'},
'DependsOn': 'dummy',
'DeletionPolicy': 'dummy',
'UpdatePolicy': {'foo': 'bar'}}}
tmpl = parser.Template(hot_tpl)
self.assertEqual(expected, tmpl[tmpl.RESOURCES])
def test_translate_outputs(self):
def test_translate_resources_bad_type(self):
"""Test translation of resources including invalid keyword."""
hot_tpl = template_format.parse('''
heat_template_version: 2013-05-23
resources:
resource1:
Type: AWS::EC2::Instance
properties:
property1: value1
metadata:
foo: bar
depends_on: dummy
deletion_policy: dummy
update_policy:
foo: bar
''')
tmpl = parser.Template(hot_tpl)
err = self.assertRaises(KeyError, tmpl.__getitem__, tmpl.RESOURCES)
self.assertIn('Type', str(err))
def test_translate_resources_bad_properties(self):
"""Test translation of resources including invalid keyword."""
hot_tpl = template_format.parse('''
heat_template_version: 2013-05-23
resources:
resource1:
type: AWS::EC2::Instance
Properties:
property1: value1
metadata:
foo: bar
depends_on: dummy
deletion_policy: dummy
update_policy:
foo: bar
''')
tmpl = parser.Template(hot_tpl)
err = self.assertRaises(KeyError, tmpl.__getitem__, tmpl.RESOURCES)
self.assertIn('Properties', str(err))
def test_translate_resources_bad_metadata(self):
"""Test translation of resources including invalid keyword."""
hot_tpl = template_format.parse('''
heat_template_version: 2013-05-23
resources:
resource1:
type: AWS::EC2::Instance
properties:
property1: value1
Metadata:
foo: bar
depends_on: dummy
deletion_policy: dummy
update_policy:
foo: bar
''')
tmpl = parser.Template(hot_tpl)
err = self.assertRaises(KeyError, tmpl.__getitem__, tmpl.RESOURCES)
self.assertIn('Metadata', str(err))
def test_translate_resources_bad_depends_on(self):
"""Test translation of resources including invalid keyword."""
hot_tpl = template_format.parse('''
heat_template_version: 2013-05-23
resources:
resource1:
type: AWS::EC2::Instance
properties:
property1: value1
metadata:
foo: bar
DependsOn: dummy
deletion_policy: dummy
update_policy:
foo: bar
''')
tmpl = parser.Template(hot_tpl)
err = self.assertRaises(KeyError, tmpl.__getitem__, tmpl.RESOURCES)
self.assertIn('DependsOn', str(err))
def test_translate_resources_bad_deletion_polciy(self):
"""Test translation of resources including invalid keyword."""
hot_tpl = template_format.parse('''
heat_template_version: 2013-05-23
resources:
resource1:
type: AWS::EC2::Instance
properties:
property1: value1
metadata:
foo: bar
depends_on: dummy
DeletionPolicy: dummy
update_policy:
foo: bar
''')
tmpl = parser.Template(hot_tpl)
err = self.assertRaises(KeyError, tmpl.__getitem__, tmpl.RESOURCES)
self.assertIn('DeletionPolicy', str(err))
def test_translate_resources_bad_update_policy(self):
"""Test translation of resources including invalid keyword."""
hot_tpl = template_format.parse('''
heat_template_version: 2013-05-23
resources:
resource1:
type: AWS::EC2::Instance
properties:
property1: value1
metadata:
foo: bar
depends_on: dummy
deletion_policy: dummy
UpdatePolicy:
foo: bar
''')
tmpl = parser.Template(hot_tpl)
err = self.assertRaises(KeyError, tmpl.__getitem__, tmpl.RESOURCES)
self.assertIn('UpdatePolicy', str(err))
def test_translate_outputs_good(self):
"""Test translation of outputs into internal engine format."""
hot_tpl = template_format.parse('''
@ -117,6 +259,36 @@ class HOTemplateTest(HeatTestCase):
tmpl = parser.Template(hot_tpl)
self.assertEqual(expected, tmpl[tmpl.OUTPUTS])
def test_translate_outputs_bad_description(self):
"""Test translation of outputs into internal engine format."""
hot_tpl = template_format.parse('''
heat_template_version: 2013-05-23
outputs:
output1:
Description: output1
value: value1
''')
tmpl = parser.Template(hot_tpl)
err = self.assertRaises(KeyError, tmpl.__getitem__, tmpl.OUTPUTS)
self.assertIn('Description', str(err))
def test_translate_outputs_bad_value(self):
"""Test translation of outputs into internal engine format."""
hot_tpl = template_format.parse('''
heat_template_version: 2013-05-23
outputs:
output1:
description: output1
Value: value1
''')
tmpl = parser.Template(hot_tpl)
err = self.assertRaises(KeyError, tmpl.__getitem__, tmpl.OUTPUTS)
self.assertIn('Value', str(err))
def test_str_replace(self):
"""Test str_replace function."""
@ -433,7 +605,7 @@ class StackTest(test_parser.StackTest):
tmpl = template.Template(
{'heat_template_version': '2013-05-23',
'resources': {'AResource': {'type': 'ResourceWithPropsType',
'Metadata': {'Bar': {'get_param': 'OS::stack_id'}},
'metadata': {'Bar': {'get_param': 'OS::stack_id'}},
'properties': {'Foo': 'abc'}}}})
self.stack = parser.Stack(self.ctx, 'update_stack_id_test', tmpl)
self.stack.store()
@ -446,7 +618,7 @@ class StackTest(test_parser.StackTest):
tmpl2 = template.Template(
{'heat_template_version': '2013-05-23',
'resources': {'AResource': {'type': 'ResourceWithPropsType',
'Metadata': {'Bar': {'get_param': 'OS::stack_id'}},
'metadata': {'Bar': {'get_param': 'OS::stack_id'}},
'properties': {'Foo': 'xyz'}}}})
updated_stack = parser.Stack(self.ctx, 'updated_stack', tmpl2)

22
heat/tests/test_resource.py
View File

@ -999,7 +999,7 @@ class ResourceDependenciesTest(HeatTestCase):
'foo': {'type': 'GenericResourceType'},
'bar': {
'type': 'ResourceWithPropsType',
'Properties': {
'properties': {
'Foo': {'get_attr': ['foo', 'bar']},
}
}
@ -1205,6 +1205,26 @@ class ResourceDependenciesTest(HeatTestCase):
self.assertIn(res, graph)
self.assertIn(stack['foo'], graph[res])
def test_dependson_hot(self):
tmpl = template.Template({
'heat_template_version': '2013-05-23',
'resources': {
'foo': {'type': 'GenericResourceType'},
'bar': {
'type': 'GenericResourceType',
'depends_on': 'foo',
}
}
})
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
res = stack['bar']
res.add_dependencies(self.deps)
graph = self.deps.graph()
self.assertIn(res, graph)
self.assertIn(stack['foo'], graph[res])
def test_dependson_fail(self):
tmpl = template.Template({
'Resources': {