Fix HTTP error codes due to invalid templates
This reverts commit 9a4d719ea1
and creates a
new exception type, InvalidGlobalResource, that is raised when a
non-existent template is referenced in the global environment and which
results in an HTTP 500 error.
This fixes the cases where we could return 404 or 500 errors as a result of
template validation failures, when we should return a 400 error.
Change-Id: I3c7b3182957448bb13514696cc2e12f5aaddd253
Closes-Bug: #1518458
Related-Bug: #1447194
This commit is contained in:
parent
17851c88c9
commit
517f91ce27
@ -60,6 +60,7 @@ class FaultWrapper(wsgi.Middleware):
|
||||
'NotFound': webob.exc.HTTPNotFound,
|
||||
'ResourceActionNotSupported': webob.exc.HTTPBadRequest,
|
||||
'ResourceNotFound': webob.exc.HTTPNotFound,
|
||||
'InvalidGlobalResource': webob.exc.HTTPInternalServerError,
|
||||
'SnapshotNotFound': webob.exc.HTTPNotFound,
|
||||
'ResourceNotAvailable': webob.exc.HTTPNotFound,
|
||||
'PhysicalResourceNameAmbiguity': webob.exc.HTTPBadRequest,
|
||||
|
@ -209,12 +209,9 @@ class SnapshotNotFound(HeatException):
|
||||
"could not be found.")
|
||||
|
||||
|
||||
class TemplateNotFound(HeatException):
|
||||
msg_fmt = _("%(message)s")
|
||||
|
||||
|
||||
class InvalidResourceType(HeatException):
|
||||
msg_fmt = _("%(message)s")
|
||||
class InvalidGlobalResource(HeatException):
|
||||
msg_fmt = _("There was an error loading the definition of the global "
|
||||
"resource type %(type_name)s.")
|
||||
|
||||
|
||||
class InvalidBreakPointHook(HeatException):
|
||||
|
@ -419,20 +419,20 @@ class ResourceRegistry(object):
|
||||
def get_class(self, resource_type, resource_name=None, files=None):
|
||||
if resource_type == "":
|
||||
msg = _('Resource "%s" has no type') % resource_name
|
||||
raise exception.InvalidResourceType(message=msg)
|
||||
raise exception.StackValidationFailed(message=msg)
|
||||
elif resource_type is None:
|
||||
msg = _('Non-empty resource type is required '
|
||||
'for resource "%s"') % resource_name
|
||||
raise exception.InvalidResourceType(message=msg)
|
||||
raise exception.StackValidationFailed(message=msg)
|
||||
elif not isinstance(resource_type, six.string_types):
|
||||
msg = _('Resource "%s" type is not a string') % resource_name
|
||||
raise exception.InvalidResourceType(message=msg)
|
||||
raise exception.StackValidationFailed(message=msg)
|
||||
|
||||
info = self.get_resource_info(resource_type,
|
||||
resource_name=resource_name)
|
||||
if info is None:
|
||||
raise exception.EntityNotFound(entity='Resource Type',
|
||||
name=resource_type)
|
||||
msg = _("Unknown resource Type : %s") % resource_type
|
||||
raise exception.StackValidationFailed(message=msg)
|
||||
return info.get_class(files=files)
|
||||
|
||||
def as_dict(self):
|
||||
|
@ -138,7 +138,7 @@ class Resource(object):
|
||||
ResourceClass = registry.get_class(definition.resource_type,
|
||||
resource_name=name,
|
||||
files=stack.t.files)
|
||||
except exception.TemplateNotFound:
|
||||
except exception.NotFound:
|
||||
ResourceClass = template_resource.TemplateResource
|
||||
|
||||
assert issubclass(ResourceClass, Resource)
|
||||
|
@ -276,7 +276,7 @@ class ResourceGroup(stack_resource.StackResource):
|
||||
# make sure we can resolve the nested resource type
|
||||
try:
|
||||
self.stack.env.get_class(res_def.resource_type)
|
||||
except exception.TemplateNotFound:
|
||||
except exception.NotFound:
|
||||
# its a template resource
|
||||
pass
|
||||
|
||||
|
@ -98,7 +98,7 @@ class TemplateResource(stack_resource.StackResource):
|
||||
args = {'name': template_name, 'exc': six.text_type(r_exc)}
|
||||
msg = _('Could not fetch remote template '
|
||||
'"%(name)s": %(exc)s') % args
|
||||
raise exception.TemplateNotFound(message=msg)
|
||||
raise exception.NotFound(msg_fmt=msg)
|
||||
|
||||
@staticmethod
|
||||
def get_schemas(tmpl, param_defaults):
|
||||
@ -111,7 +111,7 @@ class TemplateResource(stack_resource.StackResource):
|
||||
self._parsed_nested = None
|
||||
try:
|
||||
tmpl = template.Template(self.child_template())
|
||||
except (exception.TemplateNotFound, ValueError) as download_error:
|
||||
except (exception.NotFound, ValueError) as download_error:
|
||||
self.validation_exception = download_error
|
||||
tmpl = template.Template(
|
||||
{"HeatTemplateFormatVersion": "2012-12-12"})
|
||||
@ -199,7 +199,7 @@ class TemplateResource(stack_resource.StackResource):
|
||||
try:
|
||||
t_data = self.get_template_file(self.template_name,
|
||||
self.allowed_schemes)
|
||||
except exception.TemplateNotFound as err:
|
||||
except exception.NotFound as err:
|
||||
if self.action == self.UPDATE:
|
||||
raise
|
||||
reported_excp = err
|
||||
|
@ -1220,10 +1220,14 @@ class EngineService(service.Service):
|
||||
self.resource_enforcer.enforce(cnxt, type_name)
|
||||
try:
|
||||
resource_class = resources.global_env().get_class(type_name)
|
||||
except (exception.InvalidResourceType,
|
||||
exception.EntityNotFound,
|
||||
exception.TemplateNotFound) as ex:
|
||||
raise ex
|
||||
except exception.StackValidationFailed:
|
||||
raise exception.EntityNotFound(entity='Resource 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)
|
||||
|
||||
if resource_class.support_status.status == support.HIDDEN:
|
||||
raise exception.NotSupported(type_name)
|
||||
@ -1269,10 +1273,14 @@ class EngineService(service.Service):
|
||||
raise exception.NotSupported(type_name)
|
||||
return resource_class.resource_to_template(type_name,
|
||||
template_type)
|
||||
except (exception.InvalidResourceType,
|
||||
exception.EntityNotFound,
|
||||
exception.TemplateNotFound) as ex:
|
||||
raise ex
|
||||
except exception.StackValidationFailed:
|
||||
raise exception.EntityNotFound(entity='Resource 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)
|
||||
|
||||
@context.request_context
|
||||
def list_events(self, cnxt, stack_identity, filters=None, limit=None,
|
||||
|
@ -2310,6 +2310,27 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
|
||||
self.assertEqual('EntityNotFound', resp.json['error']['type'])
|
||||
self.m.VerifyAll()
|
||||
|
||||
def test_resource_schema_faulty_template(self, mock_enforce):
|
||||
self._mock_enforce_setup(mock_enforce, 'resource_schema', True)
|
||||
req = self._get('/resource_types/FaultyTemplate')
|
||||
type_name = 'FaultyTemplate'
|
||||
|
||||
error = heat_exc.InvalidGlobalResource(type_name='FaultyTemplate')
|
||||
self.m.StubOutWithMock(rpc_client.EngineClient, 'call')
|
||||
rpc_client.EngineClient.call(
|
||||
req.context,
|
||||
('resource_schema', {'type_name': type_name})
|
||||
).AndRaise(tools.to_remote_error(error))
|
||||
self.m.ReplayAll()
|
||||
|
||||
resp = tools.request_with_middleware(fault.FaultWrapper,
|
||||
self.controller.resource_schema,
|
||||
req, tenant_id=self.tenant,
|
||||
type_name=type_name)
|
||||
self.assertEqual(500, resp.json['code'])
|
||||
self.assertEqual('InvalidGlobalResource', resp.json['error']['type'])
|
||||
self.m.VerifyAll()
|
||||
|
||||
def test_resource_schema_err_denied_policy(self, mock_enforce):
|
||||
self._mock_enforce_setup(mock_enforce, 'resource_schema', False)
|
||||
req = self._get('/resource_types/BogusResourceType')
|
||||
|
@ -134,11 +134,12 @@ class ResourceTypeTest(common.HeatTestCase):
|
||||
mock_iterable = mock.MagicMock(return_value=iter([info]))
|
||||
with mock.patch('heat.engine.environment.ResourceRegistry.iterable_by',
|
||||
new=mock_iterable):
|
||||
ex = self.assertRaises(exception.TemplateNotFound,
|
||||
ex = self.assertRaises(exception.InvalidGlobalResource,
|
||||
function,
|
||||
self.ctx,
|
||||
type_name='ResourceWithWrongRefOnFile')
|
||||
msg = 'Could not fetch remote template "not_existing.yaml"'
|
||||
msg = ('There was an error loading the definition of the global '
|
||||
'resource type ResourceWithWrongRefOnFile.')
|
||||
self.assertIn(msg, six.text_type(ex))
|
||||
|
||||
def test_resource_schema_no_template_file(self):
|
||||
|
@ -614,7 +614,7 @@ class ProviderTemplateTest(common.HeatTestCase):
|
||||
env_str = {'resource_registry': {'resources': {'fred': {
|
||||
"OS::ResourceType": "some_magic.yaml"}}}}
|
||||
env = environment.Environment(env_str)
|
||||
ex = self.assertRaises(exception.TemplateNotFound, env.get_class,
|
||||
ex = self.assertRaises(exception.NotFound, env.get_class,
|
||||
'OS::ResourceType', 'fred')
|
||||
self.assertIn('Could not fetch remote template "some_magic.yaml"',
|
||||
six.text_type(ex))
|
||||
@ -934,16 +934,18 @@ class TemplateDataTest(common.HeatTestCase):
|
||||
self.res.action = self.res.UPDATE
|
||||
self.res.nested = mock.MagicMock()
|
||||
self.res.get_template_file = mock.Mock(
|
||||
side_effect=exception.TemplateNotFound(
|
||||
message='test_resource.template'))
|
||||
self.assertRaises(exception.TemplateNotFound, self.res.template_data)
|
||||
side_effect=exception.NotFound(
|
||||
msg_fmt='Could not fetch remote template '
|
||||
'"test_resource.template": file not found'))
|
||||
self.assertRaises(exception.NotFound, self.res.template_data)
|
||||
|
||||
def test_template_data_in_create_without_template_file(self):
|
||||
self.res.action = self.res.CREATE
|
||||
self.res.nested = mock.MagicMock()
|
||||
self.res.get_template_file = mock.Mock(
|
||||
side_effect=exception.TemplateNotFound(
|
||||
message='test_resource.template'))
|
||||
side_effect=exception.NotFound(
|
||||
msg_fmt='Could not fetch remote template '
|
||||
'"test_resource.template": file not found'))
|
||||
self.assertEqual('{}', self.res.template_data())
|
||||
|
||||
|
||||
|
@ -73,7 +73,7 @@ class ResourceTest(common.HeatTestCase):
|
||||
self.assertEqual(generic_rsrc.GenericResource, cls)
|
||||
|
||||
def test_get_class_noexist(self):
|
||||
self.assertRaises(exception.EntityNotFound,
|
||||
self.assertRaises(exception.StackValidationFailed,
|
||||
resources.global_env().get_class,
|
||||
'NoExistResourceType')
|
||||
|
||||
@ -173,13 +173,13 @@ class ResourceTest(common.HeatTestCase):
|
||||
def test_resource_new_err(self):
|
||||
snippet = rsrc_defn.ResourceDefinition('aresource',
|
||||
'NoExistResourceType')
|
||||
self.assertRaises(exception.EntityNotFound,
|
||||
self.assertRaises(exception.StackValidationFailed,
|
||||
resource.Resource, 'aresource', snippet, self.stack)
|
||||
|
||||
def test_resource_non_type(self):
|
||||
resource_name = 'aresource'
|
||||
snippet = rsrc_defn.ResourceDefinition(resource_name, '')
|
||||
ex = self.assertRaises(exception.InvalidResourceType,
|
||||
ex = self.assertRaises(exception.StackValidationFailed,
|
||||
resource.Resource, resource_name,
|
||||
snippet, self.stack)
|
||||
self.assertIn(_('Resource "%s" has no type') % resource_name,
|
||||
|
@ -417,10 +417,9 @@ class ResourceGroupTest(common.HeatTestCase):
|
||||
stack = utils.parse_stack(tmp)
|
||||
snip = stack.t.resource_definitions(stack)['group1']
|
||||
resg = resource_group.ResourceGroup('test', snip, stack)
|
||||
exc = self.assertRaises(exception.EntityNotFound,
|
||||
exc = self.assertRaises(exception.StackValidationFailed,
|
||||
resg.validate)
|
||||
exp_msg = 'The Resource Type (idontexist) could not be found.'
|
||||
self.assertIn(exp_msg, six.text_type(exc))
|
||||
self.assertIn('Unknown resource Type', six.text_type(exc))
|
||||
|
||||
def test_reference_attr(self):
|
||||
stack = utils.parse_stack(template2)
|
||||
|
@ -387,28 +387,22 @@ class StackResourceTest(StackResourceBaseTest):
|
||||
self.assertEqual(raise_exc_msg, six.text_type(exc))
|
||||
|
||||
def _test_validate_unknown_resource_type(self, stack_name, tmpl,
|
||||
resource_name,
|
||||
stack_resource=True):
|
||||
raise_exc_msg = 'The Resource Type (idontexist) could not be found.'
|
||||
resource_name):
|
||||
raise_exc_msg = ('Unknown resource Type : idontexist')
|
||||
stack = parser.Stack(utils.dummy_context(), stack_name, tmpl)
|
||||
rsrc = stack[resource_name]
|
||||
if stack_resource:
|
||||
exc = self.assertRaises(exception.StackValidationFailed,
|
||||
rsrc.validate)
|
||||
else:
|
||||
exc = self.assertRaises(exception.EntityNotFound,
|
||||
rsrc.validate)
|
||||
|
||||
exc = self.assertRaises(exception.StackValidationFailed,
|
||||
rsrc.validate)
|
||||
self.assertIn(raise_exc_msg, six.text_type(exc))
|
||||
|
||||
def test_validate_resource_group(self):
|
||||
# resource group validate without nested template is a normal
|
||||
# resource validation
|
||||
# test validate without nested template
|
||||
stack_name = 'validate_resource_group_template'
|
||||
t = template_format.parse(resource_group_template)
|
||||
tmpl = templatem.Template(t)
|
||||
self._test_validate_unknown_resource_type(stack_name, tmpl,
|
||||
'my_resource_group',
|
||||
stack_resource=False)
|
||||
'my_resource_group')
|
||||
|
||||
# validate with nested template
|
||||
res_prop = t['resources']['my_resource_group']['properties']
|
||||
@ -419,7 +413,7 @@ class StackResourceTest(StackResourceBaseTest):
|
||||
'my_resource_group')
|
||||
|
||||
def test_validate_heat_autoscaling_group(self):
|
||||
# Autoscaling validation is a nested stack validation
|
||||
# test validate without nested template
|
||||
stack_name = 'validate_heat_autoscaling_group_template'
|
||||
t = template_format.parse(heat_autoscaling_group_template)
|
||||
tmpl = templatem.Template(t)
|
||||
@ -881,8 +875,8 @@ class WithTemplateTest(StackResourceBaseTest):
|
||||
class RaiseLocalException(StackResourceBaseTest):
|
||||
|
||||
def test_heat_exception(self):
|
||||
local = exception.InvalidResourceType(message='test')
|
||||
self.assertRaises(exception.InvalidResourceType,
|
||||
local = exception.StackValidationFailed(message='test')
|
||||
self.assertRaises(exception.StackValidationFailed,
|
||||
self.parent_resource.raise_local_exception, local)
|
||||
|
||||
def test_messaging_timeout(self):
|
||||
@ -891,9 +885,9 @@ class RaiseLocalException(StackResourceBaseTest):
|
||||
self.parent_resource.raise_local_exception, local)
|
||||
|
||||
def test_remote_heat_ex(self):
|
||||
class InvalidResourceType_Remote(exception.InvalidResourceType):
|
||||
class StackValidationFailed_Remote(exception.StackValidationFailed):
|
||||
pass
|
||||
|
||||
local = InvalidResourceType_Remote(message='test')
|
||||
local = StackValidationFailed_Remote(message='test')
|
||||
self.assertRaises(exception.ResourceFailure,
|
||||
self.parent_resource.raise_local_exception, local)
|
||||
|
Loading…
Reference in New Issue
Block a user