openstack/heat
Code Issues Proposed changes

Remove broken heat.resource_type custom constraint

Browse Source 
This reverts the commits f5c32ad8fd and
14fdf72b000c82a80abb2587189dd7c6c7dfa0a0e.

The constraint never worked and the stuff to pass the template to
constraints (which was broken because we actually passed a
ResourceDefinition instead) is a pain. Just get rid of it rather than
fix it.

Change-Id: I4e1e787ad94ac1951f472ea066a9b1c9d3e03611
Closes-Bug: #1661403
This commit is contained in:
Zane Bitter 2017-08-01 12:54:09 -04:00
parent 8c7b66a8a3
commit c26f367b03
9 changed files with 53 additions and 198 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
heat/engine/constraint/heat_constraints.py
View File

@ -1,45 +0,0 @@
#
# 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 collections
import six
from heat.common.i18n import _
from heat.engine import constraints
class ResourceTypeConstraint(constraints.BaseCustomConstraint):
def validate(self, value, context, template=None):
if not isinstance(value, collections.Sequence):
return False
if isinstance(value, six.string_types):
value = [value]
invalid_types = []
for t in value:
try:
template.env.get_class(t)
except Exception:
invalid_types.append(t)
if invalid_types:
msg = _('The following resource types could not be found: %s')
types = ','.join(invalid_types)
self._error_message = msg % types
return False
return True

36
heat/engine/constraints.py
View File

@ -204,15 +204,14 @@ class Schema(collections.Mapping):
return value return value
def validate_constraints(self, value, context=None, skipped=None, def validate_constraints(self, value, context=None, skipped=None):
template=None):
if not skipped: if not skipped:
skipped = [] skipped = []
try: try:
for constraint in self.constraints: for constraint in self.constraints:
if type(constraint) not in skipped: if type(constraint) not in skipped:
constraint.validate(value, self, context, template) constraint.validate(value, self, context)
except ValueError as ex: except ValueError as ex:
raise exception.StackValidationFailed(message=six.text_type(ex)) raise exception.StackValidationFailed(message=six.text_type(ex))
@ -296,8 +295,8 @@ class Constraint(collections.Mapping):
return '\n'.join(desc()) return '\n'.join(desc())
def validate(self, value, schema=None, context=None, template=None): def validate(self, value, schema=None, context=None):
if not self._is_valid(value, schema, context, template): if not self._is_valid(value, schema, context):
if self.description: if self.description:
err_msg = self.description err_msg = self.description
else: else:
@ -374,7 +373,7 @@ class Range(Constraint):
self.min, self.min,
self.max) self.max)
def _is_valid(self, value, schema, context, template): def _is_valid(self, value, schema, context):
value = Schema.str_to_num(value) value = Schema.str_to_num(value)
if self.min is not None: if self.min is not None:
@ -437,9 +436,8 @@ class Length(Range):
self.min, self.min,
self.max) self.max)
def _is_valid(self, value, schema, context, template): def _is_valid(self, value, schema, context):
return super(Length, self)._is_valid(len(value), schema, context, return super(Length, self)._is_valid(len(value), schema, context)
template)
class Modulo(Constraint): class Modulo(Constraint):
@ -503,7 +501,7 @@ class Modulo(Constraint):
return '%s is not a multiple of %s with an offset of %s)' % ( return '%s is not a multiple of %s with an offset of %s)' % (
value, self.step, self.offset) value, self.step, self.offset)
def _is_valid(self, value, schema, context, template): def _is_valid(self, value, schema, context):
value = Schema.str_to_num(value) value = Schema.str_to_num(value)
if value % self.step != self.offset: if value % self.step != self.offset:
@ -551,7 +549,7 @@ class AllowedValues(Constraint):
allowed = '[%s]' % ', '.join(str(a) for a in self.allowed) allowed = '[%s]' % ', '.join(str(a) for a in self.allowed)
return '"%s" is not an allowed value %s' % (value, allowed) return '"%s" is not an allowed value %s' % (value, allowed)
def _is_valid(self, value, schema, context, template): def _is_valid(self, value, schema, context):
# For list values, check if all elements of the list are contained # For list values, check if all elements of the list are contained
# in allowed list. # in allowed list.
if isinstance(value, list): if isinstance(value, list):
@ -594,7 +592,7 @@ class AllowedPattern(Constraint):
def _err_msg(self, value): def _err_msg(self, value):
return '"%s" does not match pattern "%s"' % (value, self.pattern) return '"%s" does not match pattern "%s"' % (value, self.pattern)
def _is_valid(self, value, schema, context, template): def _is_valid(self, value, schema, context):
match = self.match(value) match = self.match(value)
return match is not None and match.end() == len(value) return match is not None and match.end() == len(value)
@ -645,19 +643,11 @@ class CustomConstraint(Constraint):
return _('"%(value)s" does not validate %(name)s') % { return _('"%(value)s" does not validate %(name)s') % {
"value": value, "name": self.name} "value": value, "name": self.name}
def _is_valid(self, value, schema, context, template): def _is_valid(self, value, schema, context):
constraint = self.custom_constraint constraint = self.custom_constraint
if not constraint: if not constraint:
return False return False
return constraint.validate(value, context)
try:
result = constraint.validate(value, context,
template=template)
except TypeError:
# for backwards compatibility with older service constraints
result = constraint.validate(value, context)
return result
class BaseCustomConstraint(object): class BaseCustomConstraint(object):
@ -679,7 +669,7 @@ class BaseCustomConstraint(object):
return _("Error validating value '%(value)s': %(message)s") % { return _("Error validating value '%(value)s': %(message)s") % {
"value": value, "message": self._error_message} "value": value, "message": self._error_message}
def validate(self, value, context, template=None): def validate(self, value, context):
@MEMOIZE @MEMOIZE
def check_cache_or_validate_value(cache_value_prefix, def check_cache_or_validate_value(cache_value_prefix,

33
heat/engine/parameters.py
View File

@ -165,9 +165,8 @@ class Schema(constr.Schema):
'false')).lower() == 'true', 'false')).lower() == 'true',
label=schema_dict.get(LABEL)) label=schema_dict.get(LABEL))
def validate_value(self, value, context=None, template=None): def validate_value(self, value, context=None):
super(Schema, self).validate_constraints(value, context=context, super(Schema, self).validate_constraints(value, context)
template=template)
def __getitem__(self, key): def __getitem__(self, key):
if key == self.TYPE: if key == self.TYPE:
@ -219,7 +218,7 @@ class Parameter(object):
self.user_value = value self.user_value = value
self.user_default = None self.user_default = None
def validate(self, validate_value=True, context=None, template=None): def validate(self, validate_value=True, context=None):
"""Validates the parameter. """Validates the parameter.
This method validates if the parameter's schema is valid, This method validates if the parameter's schema is valid,
@ -235,9 +234,9 @@ class Parameter(object):
return return
if self.user_value is not None: if self.user_value is not None:
self._validate(self.user_value, context, template) self._validate(self.user_value, context)
elif self.has_default(): elif self.has_default():
self._validate(self.default(), context, template) self._validate(self.default(), context)
else: else:
raise exception.UserParameterMissing(key=self.name) raise exception.UserParameterMissing(key=self.name)
except exception.StackValidationFailed as ex: except exception.StackValidationFailed as ex:
@ -316,12 +315,12 @@ class NumberParam(Parameter):
"""Return a float representation of the parameter.""" """Return a float representation of the parameter."""
return float(super(NumberParam, self).value()) return float(super(NumberParam, self).value())
def _validate(self, val, context, template=None): def _validate(self, val, context):
try: try:
Schema.str_to_num(val) Schema.str_to_num(val)
except (ValueError, TypeError) as ex: except (ValueError, TypeError) as ex:
raise exception.StackValidationFailed(message=six.text_type(ex)) raise exception.StackValidationFailed(message=six.text_type(ex))
self.schema.validate_value(val, context=context, template=template) self.schema.validate_value(val, context)
def value(self): def value(self):
return Schema.str_to_num(super(NumberParam, self).value()) return Schema.str_to_num(super(NumberParam, self).value())
@ -332,12 +331,12 @@ class BooleanParam(Parameter):
__slots__ = tuple() __slots__ = tuple()
def _validate(self, val, context, template=None): def _validate(self, val, context):
try: try:
strutils.bool_from_string(val, strict=True) strutils.bool_from_string(val, strict=True)
except ValueError as ex: except ValueError as ex:
raise exception.StackValidationFailed(message=six.text_type(ex)) raise exception.StackValidationFailed(message=six.text_type(ex))
self.schema.validate_value(val, context=context, template=template) self.schema.validate_value(val, context)
def value(self): def value(self):
if self.user_value is not None: if self.user_value is not None:
@ -352,8 +351,8 @@ class StringParam(Parameter):
__slots__ = tuple() __slots__ = tuple()
def _validate(self, val, context, template=None): def _validate(self, val, context):
self.schema.validate_value(val, context=context, template=template) self.schema.validate_value(val, context=context)
def value(self): def value(self):
return self.schema.to_schema_type(super(StringParam, self).value()) return self.schema.to_schema_type(super(StringParam, self).value())
@ -418,12 +417,12 @@ class CommaDelimitedListParam(ParsedParameter, collections.Sequence):
def _value_as_text(cls, value): def _value_as_text(cls, value):
return ",".join(value) return ",".join(value)
def _validate(self, val, context, template=None): def _validate(self, val, context):
try: try:
parsed = self.parse(val) parsed = self.parse(val)
except ValueError as ex: except ValueError as ex:
raise exception.StackValidationFailed(message=six.text_type(ex)) raise exception.StackValidationFailed(message=six.text_type(ex))
self.schema.validate_value(parsed, context=context, template=template) self.schema.validate_value(parsed, context)
class JsonParam(ParsedParameter): class JsonParam(ParsedParameter):
@ -467,12 +466,12 @@ class JsonParam(ParsedParameter):
def _value_as_text(cls, value): def _value_as_text(cls, value):
return encodeutils.safe_decode(jsonutils.dumps(value)) return encodeutils.safe_decode(jsonutils.dumps(value))
def _validate(self, val, context, template=None): def _validate(self, val, context):
try: try:
parsed = self.parse(val) parsed = self.parse(val)
except ValueError as ex: except ValueError as ex:
raise exception.StackValidationFailed(message=six.text_type(ex)) raise exception.StackValidationFailed(message=six.text_type(ex))
self.schema.validate_value(parsed, context=context, template=template) self.schema.validate_value(parsed, context)
@six.add_metaclass(abc.ABCMeta) @six.add_metaclass(abc.ABCMeta)
@ -525,7 +524,7 @@ class Parameters(collections.Mapping):
self._validate_user_parameters() self._validate_user_parameters()
for param in six.itervalues(self.params): for param in six.itervalues(self.params):
param.validate(validate_value, context, self.tmpl) param.validate(validate_value, context)
def __contains__(self, key): def __contains__(self, key):
"""Return whether the specified parameter exists.""" """Return whether the specified parameter exists."""

25
heat/engine/properties.py
View File

@ -349,8 +349,7 @@ class Property(object):
raise TypeError(_('"%s" is not a valid boolean') % value) raise TypeError(_('"%s" is not a valid boolean') % value)
def get_value(self, value, validate=False, template=None, def get_value(self, value, validate=False, translation=None):
translation=None):
"""Get value from raw value and sanitize according to data type.""" """Get value from raw value and sanitize according to data type."""
t = self.type() t = self.type()
@ -370,8 +369,7 @@ class Property(object):
_value = value _value = value
if validate: if validate:
self.schema.validate_constraints(_value, self.context, self.schema.validate_constraints(_value, self.context)
template=template)
return _value return _value
@ -405,7 +403,7 @@ class Properties(collections.Mapping):
in params_snippet.items()) in params_snippet.items())
return {} return {}
def validate(self, with_value=True, template=None): def validate(self, with_value=True):
try: try:
for key in self.data: for key in self.data:
if key not in self.props: if key not in self.props:
@ -418,9 +416,7 @@ class Properties(collections.Mapping):
continue continue
if with_value: if with_value:
try: try:
self._get_property_value(key, self._get_property_value(key, validate=True)
validate=True,
template=template)
except exception.StackValidationFailed as ex: except exception.StackValidationFailed as ex:
path = [key] path = [key]
path.extend(ex.path) path.extend(ex.path)
@ -455,7 +451,7 @@ class Properties(collections.Mapping):
if any(res.action == res.INIT for res in deps): if any(res.action == res.INIT for res in deps):
return True return True
def get_user_value(self, key, validate=False, template=None): def get_user_value(self, key, validate=False):
if key not in self: if key not in self:
raise KeyError(_('Invalid Property %s') % key) raise KeyError(_('Invalid Property %s') % key)
@ -477,7 +473,7 @@ class Properties(collections.Mapping):
value, value,
self.data) self.data)
return prop.get_value(value, validate, template=template, return prop.get_value(value, validate,
translation=self.translation) translation=self.translation)
# Children can raise StackValidationFailed with unique path which # Children can raise StackValidationFailed with unique path which
# is necessary for further use in StackValidationFailed exception. # is necessary for further use in StackValidationFailed exception.
@ -490,23 +486,22 @@ class Properties(collections.Mapping):
except Exception as e: except Exception as e:
raise ValueError(six.text_type(e)) raise ValueError(six.text_type(e))
def _get_property_value(self, key, validate=False, template=None): def _get_property_value(self, key, validate=False):
if key not in self: if key not in self:
raise KeyError(_('Invalid Property %s') % key) raise KeyError(_('Invalid Property %s') % key)
prop = self.props[key] prop = self.props[key]
if not self.translation.is_deleted(prop.path) and key in self.data: if not self.translation.is_deleted(prop.path) and key in self.data:
return self.get_user_value(key, validate, template=template) return self.get_user_value(key, validate)
elif self.translation.has_translation(prop.path): elif self.translation.has_translation(prop.path):
value = self.translation.translate(prop.path, prop_data=self.data, value = self.translation.translate(prop.path, prop_data=self.data,
validate=validate, validate=validate)
template=template)
if value is not None or prop.has_default(): if value is not None or prop.has_default():
return prop.get_value(value) return prop.get_value(value)
elif prop.required(): elif prop.required():
raise ValueError(_('Property %s not assigned') % key) raise ValueError(_('Property %s not assigned') % key)
elif prop.has_default(): elif prop.has_default():
return prop.get_value(None, validate, template=template, return prop.get_value(None, validate,
translation=self.translation) translation=self.translation)
elif prop.required(): elif prop.required():
raise ValueError(_('Property %s not assigned') % key) raise ValueError(_('Property %s not assigned') % key)

3
heat/engine/resource.py
View File

@ -1755,8 +1755,7 @@ class Resource(status.ResourceStatus):
self.context).validate() self.context).validate()
try: try:
validate = self.properties.validate( validate = self.properties.validate(
with_value=self.stack.strict_validate, with_value=self.stack.strict_validate)
template=self.t)
except exception.StackValidationFailed as ex: except exception.StackValidationFailed as ex:
path = [self.stack.t.RESOURCES, self.t.name, path = [self.stack.t.RESOURCES, self.t.name,
self.stack.t.get_section_name(ex.path[0])] self.stack.t.get_section_name(ex.path[0])]

22
heat/engine/translation.py
View File

@ -197,8 +197,7 @@ class Translation(object):
return (self.is_active and return (self.is_active and
(key in self._rules or key in self.resolved_translations)) (key in self._rules or key in self.resolved_translations))
def translate(self, key, prop_value=None, prop_data=None, validate=False, def translate(self, key, prop_value=None, prop_data=None, validate=False):
template=None):
if key in self.resolved_translations: if key in self.resolved_translations:
return self.resolved_translations[key] return self.resolved_translations[key]
@ -212,12 +211,10 @@ class Translation(object):
result = None result = None
if rule.rule == TranslationRule.REPLACE: if rule.rule == TranslationRule.REPLACE:
result = self.replace(key, rule, result, prop_data, validate, result = self.replace(key, rule, result, prop_data, validate)
template)
if rule.rule == TranslationRule.ADD: if rule.rule == TranslationRule.ADD:
result = self.add(key, rule, result, prop_data, validate, result = self.add(key, rule, result, prop_data, validate)
template)
if rule.rule == TranslationRule.RESOLVE: if rule.rule == TranslationRule.RESOLVE:
resolved_value = resolve_and_find(result, resolved_value = resolve_and_find(result,
@ -231,7 +228,7 @@ class Translation(object):
return result return result
def add(self, key, add_rule, prop_value=None, prop_data=None, def add(self, key, add_rule, prop_value=None, prop_data=None,
validate=False, template=None): validate=False):
value_path = add_rule.get_value_absolute_path() value_path = add_rule.get_value_absolute_path()
if prop_value is None: if prop_value is None:
prop_value = [] prop_value = []
@ -252,8 +249,7 @@ class Translation(object):
value = get_value(value_path, value = get_value(value_path,
prop_data if add_rule.value_name else prop_data if add_rule.value_name else
self.properties, self.properties,
validate, validate)
template)
self.is_active = True self.is_active = True
if value is not None: if value is not None:
translation_value.extend(value if isinstance(value, list) translation_value.extend(value if isinstance(value, list)
@ -264,7 +260,7 @@ class Translation(object):
return translation_value return translation_value
def replace(self, key, replace_rule, prop_value=None, prop_data=None, def replace(self, key, replace_rule, prop_value=None, prop_data=None,
validate=False, template=None): validate=False):
value = None value = None
value_path = replace_rule.get_value_absolute_path(full_value_name=True) value_path = replace_rule.get_value_absolute_path(full_value_name=True)
short_path = replace_rule.get_value_absolute_path() short_path = replace_rule.get_value_absolute_path()
@ -280,7 +276,7 @@ class Translation(object):
subpath = value_path subpath = value_path
props = prop_data if replace_rule.value_name else self.properties props = prop_data if replace_rule.value_name else self.properties
self.is_active = False self.is_active = False
value = get_value(subpath, props, validate, template) value = get_value(subpath, props, validate)
self.is_active = True self.is_active = True
if self.has_translation(prop_path): if self.has_translation(prop_path):
@ -304,7 +300,7 @@ class Translation(object):
return result return result
def get_value(path, props, validate=False, template=None): def get_value(path, props, validate=False):
if not props: if not props:
return None return None
@ -312,7 +308,7 @@ def get_value(path, props, validate=False, template=None):
if isinstance(props, dict): if isinstance(props, dict):
prop = props.get(key) prop = props.get(key)
else: else:
prop = props._get_property_value(key, validate, template) prop = props._get_property_value(key, validate)
if len(path[1:]) == 0: if len(path[1:]) == 0:
return prop return prop
elif prop is None: elif prop is None:

82
heat/tests/constraints/test_heat_constraints.py
View File

@ -1,82 +0,0 @@
#
# 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 mock
from heat.engine.constraint import heat_constraints as hc
from heat.tests import common
class ResourceTypeConstraintTest(common.HeatTestCase):
def setUp(self):
super(ResourceTypeConstraintTest, self).setUp()
self.constraint = hc.ResourceTypeConstraint()
self.mock_template = mock.MagicMock()
self.mock_env = mock.MagicMock()
self.mock_template.env = self.mock_env
def test_validate(self):
# Setup
value = ['OS::Heat::None']
# Test
result = self.constraint.validate(value, None, self.mock_template)
# Verify
self.assertTrue(result)
self.mock_env.get_class.assert_called_once_with(value[0])
def test_validate_failure(self):
# Setup
value = ['OS::Heat::None']
self.mock_env.get_class.side_effect = Exception()
# Test
result = self.constraint.validate(value, None, self.mock_template)
# Verify
self.assertFalse(result)
self.assertIn('OS::Heat::None', self.constraint._error_message)
self.mock_env.get_class.assert_called_once_with(value[0])
def test_validate_multiple_failures(self):
# Setup
value = ['OS::Heat::None', 'OS::Heat::RandomString']
self.mock_env.get_class.side_effect = [Exception(), Exception()]
# Test
result = self.constraint.validate(value, None, self.mock_template)
# Verify
self.assertFalse(result)
self.assertIn('OS::Heat::None,OS::Heat::RandomString',
self.constraint._error_message)
self.mock_env.get_class.assert_has_calls([mock.call(value[0]),
mock.call(value[1])])
def test_validate_single_item(self):
# Setup
value = 'OS::Heat::None'
# Test
result = self.constraint.validate(value, None, self.mock_template)
# Verify
self.assertTrue(result)
self.mock_env.get_class.assert_called_once_with(value)
def test_validate_non_string(self):
result = self.constraint.validate(dict(), None, self.mock_template)
self.assertFalse(result)

4
releasenotes/notes/remove-heat-resourcetype-constraint-b679618a149fc04e.yaml Normal file
View File

@ -0,0 +1,4 @@
---
deprecations:
- The heat.resource_type custom constraint has been removed. This constraint
never actually worked.

1
setup.cfg
View File

@ -104,7 +104,6 @@ heat.constraints =
designate.domain = heat.engine.clients.os.designate:DesignateDomainConstraint designate.domain = heat.engine.clients.os.designate:DesignateDomainConstraint
designate.zone = heat.engine.clients.os.designate:DesignateZoneConstraint designate.zone = heat.engine.clients.os.designate:DesignateZoneConstraint
glance.image = heat.engine.clients.os.glance:ImageConstraint glance.image = heat.engine.clients.os.glance:ImageConstraint
heat.resource_type = heat.engine.constraint.heat_constraints:ResourceTypeConstraint
keystone.domain = heat.engine.clients.os.keystone.keystone_constraints:KeystoneDomainConstraint keystone.domain = heat.engine.clients.os.keystone.keystone_constraints:KeystoneDomainConstraint
keystone.group = heat.engine.clients.os.keystone.keystone_constraints:KeystoneGroupConstraint keystone.group = heat.engine.clients.os.keystone.keystone_constraints:KeystoneGroupConstraint
keystone.project = heat.engine.clients.os.keystone.keystone_constraints:KeystoneProjectConstraint keystone.project = heat.engine.clients.os.keystone.keystone_constraints:KeystoneProjectConstraint