From 278c5943f51c9bb4e3c23e4eaabead2da0ab4e9c Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Tue, 24 Nov 2015 12:29:38 -0500 Subject: [PATCH] Load template files only from their known source Modify get_class to ensure that user-defined resources cannot result in reads from the local filesystem. Only resources defined by the operator in the global environment should read local files. To make this work, this patch also adds a separate get_class_to_instantiate() method to the Environment. We were previously using get_class for two different purposes - to get a resource plugin on which we could perform introspection to obtain the properties and attributes schema, and to get a resource plugin we could instantiate to create a Resource object. These are both the same except in the case of a TemplateResource, where having two different use cases for the same piece of code was adding considerable extra complexity. Combining the use cases in this way also made the error handling confusing (leading to bug 1518458). This change separates out the two cases. Change-Id: I845e7d23c73242a4a4c9c40599690ab705c75caa Closes-Bug: #1496277 Related-Bug: #1447194 Related-Bug: #1518458 Related-Bug: #1508115 (cherry picked from commit 06a713c4456203cd561f16721dc8ac3bcbb37a3 and 208c0a71996faf533d9836c9e5b42ce6ecdd4fd1) --- heat/engine/environment.py | 38 +++++++++++++++++++--- heat/engine/resource.py | 11 ++----- .../resources/openstack/heat/resource_group.py | 6 +--- heat/engine/resources/template_resource.py | 15 ++++----- heat/engine/service.py | 13 +++----- heat/tests/test_provider_template.py | 10 +++--- heat/tests/test_resource.py | 5 +-- 7 files changed, 58 insertions(+), 40 deletions(-) diff --git a/heat/engine/environment.py b/heat/engine/environment.py index 0fd51b3..af8b328 100644 --- a/heat/engine/environment.py +++ b/heat/engine/environment.py @@ -118,6 +118,12 @@ class ResourceInfo(object): def matches(self, resource_type): return False + def get_class(self): + raise NotImplemented + + def get_class_to_instantiate(self): + return self.get_class() + def __str__(self): return '[%s](User:%s) %s -> %s' % (self.description, self.user_resource, @@ -146,10 +152,23 @@ class TemplateResourceInfo(ResourceInfo): def get_class(self, files=None): from heat.engine.resources import template_resource + if files and self.template_name in files: + data = files[self.template_name] + else: + if self.user_resource: + allowed_schemes = template_resource.REMOTE_SCHEMES + else: + allowed_schemes = template_resource.LOCAL_SCHEMES + data = template_resource.TemplateResource.get_template_file( + self.template_name, + allowed_schemes) env = self.registry.environment - return template_resource.generate_class(str(self.name), - self.template_name, - env, files=files) + return template_resource.generate_class_from_template(str(self.name), + data, env) + + def get_class_to_instantiate(self): + from heat.engine.resources import template_resource + return template_resource.TemplateResource class MapResourceInfo(ResourceInfo): @@ -418,6 +437,13 @@ class ResourceRegistry(object): return match def get_class(self, resource_type, resource_name=None, files=None): + info = self.get_resource_info(resource_type, + resource_name=resource_name) + if info is None: + raise exception.ResourceTypeNotFound(type_name=resource_type) + return info.get_class(files=files) + + def get_class_to_instantiate(self, resource_type, resource_name=None): if resource_type == "": msg = _('Resource "%s" has no type') % resource_name raise exception.StackValidationFailed(message=msg) @@ -434,7 +460,7 @@ class ResourceRegistry(object): if info is None: msg = _("Unknown resource Type : %s") % resource_type raise exception.StackValidationFailed(message=msg) - return info.get_class(files=files) + return info.get_class_to_instantiate() def as_dict(self): """Return user resources in a dict format.""" @@ -582,6 +608,10 @@ class Environment(object): return self.registry.get_class(resource_type, resource_name, files=files) + def get_class_to_instantiate(self, resource_type, resource_name=None): + return self.registry.get_class_to_instantiate(resource_type, + resource_name) + def get_types(self, cnxt=None, support_status=None, diff --git a/heat/engine/resource.py b/heat/engine/resource.py index e820692..0db8c40 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -128,15 +128,10 @@ class Resource(object): # Call is already for a subclass, so pass it through ResourceClass = cls else: - from heat.engine.resources import template_resource - registry = stack.env.registry - try: - ResourceClass = registry.get_class(definition.resource_type, - resource_name=name, - files=stack.t.files) - except exception.NotFound: - ResourceClass = template_resource.TemplateResource + ResourceClass = registry.get_class_to_instantiate( + definition.resource_type, + resource_name=name) assert issubclass(ResourceClass, Resource) diff --git a/heat/engine/resources/openstack/heat/resource_group.py b/heat/engine/resources/openstack/heat/resource_group.py index c0c5555..f28dfe0 100644 --- a/heat/engine/resources/openstack/heat/resource_group.py +++ b/heat/engine/resources/openstack/heat/resource_group.py @@ -282,11 +282,7 @@ class ResourceGroup(stack_resource.StackResource): val_templ = template.Template(test_tmpl) res_def = val_templ.resource_definitions(self.stack)["0"] # make sure we can resolve the nested resource type - try: - self.stack.env.get_class(res_def.resource_type) - except exception.NotFound: - # its a template resource - pass + self.stack.env.get_class_to_instantiate(res_def.resource_type) try: name = "%s-%s" % (self.stack.name, self.name) diff --git a/heat/engine/resources/template_resource.py b/heat/engine/resources/template_resource.py index fddb813..eae54ad 100644 --- a/heat/engine/resources/template_resource.py +++ b/heat/engine/resources/template_resource.py @@ -27,12 +27,11 @@ from heat.engine.resources import stack_resource from heat.engine import template -def generate_class(name, template_name, env, files=None): - data = None - if files is not None: - data = files.get(template_name) - if data is None: - data = TemplateResource.get_template_file(template_name, ('file',)) +REMOTE_SCHEMES = ('http', 'https') +LOCAL_SCHEMES = ('file',) + + +def generate_class_from_template(name, data, env): tmpl = template.Template(template_format.parse(data)) props, attrs = TemplateResource.get_schemas(tmpl, env.param_defaults) cls = type(name, (TemplateResource,), @@ -85,9 +84,9 @@ class TemplateResource(stack_resource.StackResource): self.resource_type = tri.name self.resource_path = tri.path if tri.user_resource: - self.allowed_schemes = ('http', 'https') + self.allowed_schemes = REMOTE_SCHEMES else: - self.allowed_schemes = ('http', 'https', 'file') + self.allowed_schemes = REMOTE_SCHEMES + LOCAL_SCHEMES return tri diff --git a/heat/engine/service.py b/heat/engine/service.py index 5706451..e1b7cc0 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -1212,8 +1212,6 @@ class EngineService(service.Service): self.resource_enforcer.enforce(cnxt, type_name) try: resource_class = resources.global_env().get_class(type_name) - except exception.StackValidationFailed: - raise exception.ResourceTypeNotFound(type_name=type_name) except exception.NotFound: LOG.exception(_LE('Error loading resource type %s ' 'from global environment.'), @@ -1260,17 +1258,16 @@ class EngineService(service.Service): self.resource_enforcer.enforce(cnxt, type_name) try: resource_class = resources.global_env().get_class(type_name) - if resource_class.support_status.status == support.HIDDEN: - raise exception.NotSupported(type_name) - return resource_class.resource_to_template(type_name, - template_type) - except exception.StackValidationFailed: - raise exception.ResourceTypeNotFound(type_name=type_name) except exception.NotFound: LOG.exception(_LE('Error loading resource type %s ' 'from global environment.'), type_name) raise exception.InvalidGlobalResource(type_name=type_name) + else: + if resource_class.support_status.status == support.HIDDEN: + raise exception.NotSupported(type_name) + return resource_class.resource_to_template(type_name, + template_type) @context.request_context def list_events(self, cnxt, stack_identity, filters=None, limit=None, diff --git a/heat/tests/test_provider_template.py b/heat/tests/test_provider_template.py index d84726c..eca767f 100644 --- a/heat/tests/test_provider_template.py +++ b/heat/tests/test_provider_template.py @@ -665,7 +665,11 @@ class ProviderTemplateTest(common.HeatTestCase): env_str = {'resource_registry': {'resources': {'fred': { "OS::ResourceType": test_templ_name}}}} - env = environment.Environment(env_str) + global_env = environment.Environment({}, user_env=False) + global_env.load(env_str) + with mock.patch('heat.engine.resources._environment', + global_env): + env = environment.Environment({}) cls = env.get_class('OS::ResourceType', 'fred') self.assertNotEqual(template_resource.TemplateResource, cls) self.assertTrue(issubclass(cls, template_resource.TemplateResource)) @@ -692,10 +696,6 @@ class ProviderTemplateTest(common.HeatTestCase): self.assertTrue(test_templ, "Empty test template") self.m.StubOutWithMock(urlfetch, "get") urlfetch.get(test_templ_name, - allowed_schemes=('file',) - ).AndRaise(urlfetch.URLFetchError( - _('Failed to retrieve template'))) - urlfetch.get(test_templ_name, allowed_schemes=('http', 'https')).AndReturn(test_templ) parsed_test_templ = template_format.parse(test_templ) self.m.ReplayAll() diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index cb70bac..2909627 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -69,12 +69,13 @@ class ResourceTest(common.HeatTestCase): self.dummy_timeout = 10 def test_get_class_ok(self): - cls = resources.global_env().get_class('GenericResourceType') + cls = resources.global_env().get_class_to_instantiate( + 'GenericResourceType') self.assertEqual(generic_rsrc.GenericResource, cls) def test_get_class_noexist(self): self.assertRaises(exception.StackValidationFailed, - resources.global_env().get_class, + resources.global_env().get_class_to_instantiate, 'NoExistResourceType') def test_resource_new_ok(self): -- 2.4.3