From cafd53be11f8e556fcb19e7a35f9a426852eb1b0 Mon Sep 17 00:00:00 2001 From: tyagi Date: Mon, 13 Apr 2015 10:20:46 -0700 Subject: [PATCH] Add type field to the resource attributes schema Add data type for the attributes. This will help 1) validating the attibute against its type 2) resource-schema api will show the attribute type 3) template authors will know what type of value to expect making indexing and mapping easier 4) generating docs for the attribute type Implements: blueprint add-type-in-attributes-schema Change-Id: Ifc92c57ec1ddd2ab5f810587a1d33e762308dd8a --- heat/engine/attributes.py | 43 +++++++++++++++++-- .../engine/resources/openstack/nova/server.py | 24 +++++++---- heat/engine/service.py | 2 +- heat/tests/generic_resource.py | 15 +++++++ heat/tests/test_attributes.py | 38 ++++++++++++++++ heat/tests/test_engine_service.py | 18 ++++++++ heat/tests/test_resource.py | 19 ++++++++ 7 files changed, 147 insertions(+), 12 deletions(-) diff --git a/heat/engine/attributes.py b/heat/engine/attributes.py index 44f01e0510..16e70901fb 100644 --- a/heat/engine/attributes.py +++ b/heat/engine/attributes.py @@ -20,6 +20,10 @@ from heat.common.i18n import _ from heat.engine import constraints as constr from heat.engine import support +from oslo_log import log as logging + +LOG = logging.getLogger(__name__) + class Schema(constr.Schema): """ @@ -30,9 +34,9 @@ class Schema(constr.Schema): """ KEYS = ( - DESCRIPTION, + DESCRIPTION, TYPE ) = ( - 'description', + 'description', 'type', ) CACHE_MODES = ( @@ -43,18 +47,30 @@ class Schema(constr.Schema): 'cache_none' ) + TYPES = ( + STRING, MAP, LIST, + ) = ( + 'String', 'Map', 'List', + ) + def __init__(self, description=None, support_status=support.SupportStatus(), - cache_mode=CACHE_LOCAL): + cache_mode=CACHE_LOCAL, + type=None): self.description = description self.support_status = support_status self.cache_mode = cache_mode + self.type = type def __getitem__(self, key): if key == self.DESCRIPTION: if self.description is not None: return self.description + elif key == self.TYPE: + if self.type is not None: + return self.type.lower() + raise KeyError(key) @classmethod @@ -156,6 +172,24 @@ class Attributes(collections.Mapping): for k, v in json_snippet.items()) return {} + def _validate_type(self, attrib, value): + if attrib.schema.type == attrib.schema.STRING: + if not isinstance(value, six.string_types): + LOG.warn(_("Attribute %(name)s is not of type %(att_type)s"), + {'name': attrib.name, + 'att_type': attrib.schema.STRING}) + elif attrib.schema.type == attrib.schema.LIST: + if (not isinstance(value, collections.Sequence) + or isinstance(value, six.string_types)): + LOG.warn(_("Attribute %(name)s is not of type %(att_type)s"), + {'name': attrib.name, + 'att_type': attrib.schema.LIST}) + elif attrib.schema.type == attrib.schema.MAP: + if not isinstance(value, collections.Mapping): + LOG.warn(_("Attribute %(name)s is not of type %(att_type)s"), + {'name': attrib.name, + 'att_type': attrib.schema.MAP}) + def __getitem__(self, key): if key not in self: raise KeyError(_('%(resource)s: Invalid attribute %(key)s') % @@ -169,7 +203,10 @@ class Attributes(collections.Mapping): return self._resolved_values[key] value = self._resolver(key) + if value is not None: + # validate the value against its type + self._validate_type(attrib, value) # only store if not None, it may resolve to an actual value # on subsequent calls self._resolved_values[key] = value diff --git a/heat/engine/resources/openstack/nova/server.py b/heat/engine/resources/openstack/nova/server.py index 455ed76e2e..e128ca7e3a 100644 --- a/heat/engine/resources/openstack/nova/server.py +++ b/heat/engine/resources/openstack/nova/server.py @@ -430,19 +430,23 @@ class Server(stack_user.StackUser): attributes_schema = { NAME_ATTR: attributes.Schema( - _('Name of the server.') + _('Name of the server.'), + type=attributes.Schema.STRING ), SHOW: attributes.Schema( - _('A dict of all server details as returned by the API.') + _('A dict of all server details as returned by the API.'), + type=attributes.Schema.MAP ), ADDRESSES: attributes.Schema( _('A dict of all network addresses with corresponding port_id. ' 'The port ID may be obtained through the following expression: ' - '"{get_attr: [, addresses, , 0, port]}".') + '"{get_attr: [, addresses, , 0, port]}".'), + type=attributes.Schema.MAP ), NETWORKS_ATTR: attributes.Schema( _('A dict of assigned network addresses of the form: ' - '{"public": [ip1, ip2...], "private": [ip3, ip4]}.') + '{"public": [ip1, ip2...], "private": [ip3, ip4]}.'), + type=attributes.Schema.MAP ), FIRST_ADDRESS: attributes.Schema( _('Convenience attribute to fetch the first assigned network ' @@ -458,15 +462,18 @@ class Server(stack_user.StackUser): ) ), INSTANCE_NAME: attributes.Schema( - _('AWS compatible instance name.') + _('AWS compatible instance name.'), + type=attributes.Schema.STRING ), ACCESSIPV4: attributes.Schema( _('The manually assigned alternative public IPv4 address ' - 'of the server.') + 'of the server.'), + type=attributes.Schema.STRING ), ACCESSIPV6: attributes.Schema( _('The manually assigned alternative public IPv6 address ' - 'of the server.') + 'of the server.'), + type=attributes.Schema.STRING ), CONSOLE_URLS: attributes.Schema( _("URLs of server's consoles. " @@ -475,7 +482,8 @@ class Server(stack_user.StackUser): "e.g. get_attr: [ , console_urls, novnc ]. " "Currently supported types are " "novnc, xvpvnc, spice-html5, rdp-html5, serial."), - support_status=support.SupportStatus(version='2015.1') + support_status=support.SupportStatus(version='2015.1'), + type=attributes.Schema.MAP ), } diff --git a/heat/engine/service.py b/heat/engine/service.py index 33cee1c9bb..7cb00303bd 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -1020,7 +1020,7 @@ class EngineService(service.Service): def attributes_schema(): for name, schema_data in resource_class.attributes_schema.items(): schema = attributes.Schema.from_attribute(schema_data) - yield name, {schema.DESCRIPTION: schema.description} + yield name, dict(schema) return { rpc_api.RES_SCHEMA_RES_TYPE: type_name, diff --git a/heat/tests/generic_resource.py b/heat/tests/generic_resource.py index 456e67dab3..c1fc6969b1 100644 --- a/heat/tests/generic_resource.py +++ b/heat/tests/generic_resource.py @@ -162,3 +162,18 @@ class ResourceWithCustomConstraint(GenericResource): 'Foo': properties.Schema( properties.Schema.STRING, constraints=[constraints.CustomConstraint('neutron.network')])} + + +class ResourceWithAttributeType(GenericResource): + attributes_schema = { + 'attr1': attributes.Schema('A generic attribute', + type=attributes.Schema.STRING), + 'attr2': attributes.Schema('Another generic attribute', + type=attributes.Schema.MAP) + } + + def _resolve_attribute(self, name): + if name == 'attr1': + return "valid_sting" + elif name == 'attr2': + return "invalid_type" diff --git a/heat/tests/test_attributes.py b/heat/tests/test_attributes.py index 18f60e800a..16d8e0b776 100644 --- a/heat/tests/test_attributes.py +++ b/heat/tests/test_attributes.py @@ -26,6 +26,12 @@ class AttributeSchemaTest(common.HeatTestCase): s = attributes.Schema('A attribute') self.assertEqual(d, dict(s)) + d = {'description': 'Another attribute', + 'type': 'string'} + s = attributes.Schema('Another attribute', + type=attributes.Schema.STRING) + self.assertEqual(d, dict(s)) + def test_all_resource_schemata(self): for resource_type in resources.global_env().get_types(): for schema in six.itervalues(getattr(resource_type, @@ -39,6 +45,12 @@ class AttributeSchemaTest(common.HeatTestCase): self.assertEqual('Test description.', attributes.Schema.from_attribute(s).description) + s = attributes.Schema('Test description.', + type=attributes.Schema.MAP) + self.assertIs(s, attributes.Schema.from_attribute(s)) + self.assertEqual(attributes.Schema.MAP, + attributes.Schema.from_attribute(s).type) + def test_schema_support_status(self): schema = { 'foo_sup': attributes.Schema( @@ -176,3 +188,29 @@ class AttributesTest(common.HeatTestCase): self.assertEqual("value3", attribs['test3']) value = 'value3 changed' self.assertEqual("value3 changed", attribs['test3']) + + def test_validate_type_invalid(self): + resolver = mock.Mock() + # Test invalid string type attribute + attr_schema = attributes.Schema("Test attribute", + type=attributes.Schema.STRING) + attr = attributes.Attribute("test1", attr_schema) + attribs = attributes.Attributes('test resource', attr_schema, resolver) + attribs._validate_type(attr, []) + self.assertIn("Attribute test1 is not of type String", self.LOG.output) + + # Test invalid list type attribute + attr_schema = attributes.Schema("Test attribute", + type=attributes.Schema.LIST) + attr = attributes.Attribute("test1", attr_schema) + attribs = attributes.Attributes('test resource', attr_schema, resolver) + attribs._validate_type(attr, 'invalid') + self.assertIn("Attribute test1 is not of type List", self.LOG.output) + + # Test invalid map type attribute + attr_schema = attributes.Schema("Test attribute", + type=attributes.Schema.MAP) + attr = attributes.Attribute("test1", attr_schema) + attribs = attributes.Attributes('test resource', attr_schema, resolver) + attribs._validate_type(attr, 'invalid') + self.assertIn("Attribute test1 is not of type Map", self.LOG.output) diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py index c354557702..cd84b31060 100644 --- a/heat/tests/test_engine_service.py +++ b/heat/tests/test_engine_service.py @@ -2194,6 +2194,24 @@ class StackServiceTest(common.HeatTestCase): schema = self.eng.resource_schema(self.ctx, type_name=type_name) self.assertEqual(expected, schema) + def test_resource_schema_with_attr_type(self): + res._register_class('ResourceWithAttributeType', + generic_rsrc.ResourceWithAttributeType) + + type_name = 'ResourceWithAttributeType' + expected = { + 'resource_type': type_name, + 'properties': {}, + 'attributes': { + 'attr1': {'description': 'A generic attribute', + 'type': 'string'}, + 'attr2': {'description': 'Another generic attribute', + 'type': 'map'}, + }, + } + schema = self.eng.resource_schema(self.ctx, type_name=type_name) + self.assertEqual(expected, schema) + def _no_template_file(self, function): env = environment.Environment() info = environment.ResourceInfo(env.registry, diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index 9782fa2beb..959e9a1211 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -1285,6 +1285,25 @@ class ResourceTest(common.HeatTestCase): property_schema_name, property_schema, res_class.__name__) + def test_getatt_invalid_type(self): + resource._register_class('ResourceWithAttributeType', + generic_rsrc.ResourceWithAttributeType) + + tmpl = template.Template({ + 'heat_template_version': '2013-05-23', + 'resources': { + 'res': { + 'type': 'ResourceWithAttributeType' + } + } + }) + stack = parser.Stack(utils.dummy_context(), 'test', tmpl) + res = stack['res'] + self.assertEqual('valid_sting', res.FnGetAtt('attr1')) + + res.FnGetAtt('attr2') + self.assertIn("Attribute attr2 is not of type Map", self.LOG.output) + class ResourceAdoptTest(common.HeatTestCase): def setUp(self):