From a29ccdcdb0c2e509d3037512e5ca837ee12889a0 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Wed, 25 Jul 2018 11:39:14 -0400 Subject: [PATCH] Handle unicode in constraints Ensure that if the user provides non-ASCII descriptions or e.g. allowed values in a template, that we can print them correctly wherever they appear in API output (such as in error messages). Also allow all default error messages to be localised. Change-Id: Id2c309a33634b35a4f1f8b7ddf252db22bc46625 Story: #2003096 Task: 23188 --- heat/engine/constraints.py | 42 ++++++++++++------- .../openstack/aodh/test_composite_alarm.py | 2 +- heat/tests/openstack/glance/test_image.py | 18 +++----- heat/tests/openstack/manila/test_share.py | 2 +- .../neutron/test_neutron_rbac_policy.py | 6 +-- heat/tests/test_constraints.py | 12 +++--- heat/tests/test_validate.py | 6 +-- .../constraints-i18n-dc8b2652b8455196.yaml | 7 ++++ 8 files changed, 52 insertions(+), 43 deletions(-) create mode 100644 releasenotes/notes/constraints-i18n-dc8b2652b8455196.yaml diff --git a/heat/engine/constraints.py b/heat/engine/constraints.py index d1da790e53..6e068448d8 100644 --- a/heat/engine/constraints.py +++ b/heat/engine/constraints.py @@ -12,6 +12,7 @@ # under the License. import collections +import json import numbers import re @@ -196,7 +197,8 @@ class Schema(collections.Mapping): elif self.type == self.STRING: return six.text_type(value) elif self.type == self.BOOLEAN: - return strutils.bool_from_string(str(value), strict=True) + return strutils.bool_from_string(six.text_type(value), + strict=True) except ValueError: raise ValueError(_('Value "%(val)s" is invalid for data type ' '"%(type)s".') @@ -264,7 +266,7 @@ class AnyIndexDict(collections.Mapping): def __getitem__(self, key): if key != self.ANYTHING and not isinstance(key, six.integer_types): - raise KeyError(_('Invalid key %s') % str(key)) + raise KeyError(_('Invalid key %s') % key) return self.value @@ -275,6 +277,7 @@ class AnyIndexDict(collections.Mapping): return 1 +@six.python_2_unicode_compatible class Constraint(collections.Mapping): """Parent class for constraints on allowable values for a Property. @@ -293,7 +296,7 @@ class Constraint(collections.Mapping): yield self.description yield self._str() - return '\n'.join(desc()) + return u'\n'.join(desc()) def validate(self, value, schema=None, context=None): if not self._is_valid(value, schema, context): @@ -369,9 +372,10 @@ class Range(Constraint): return fmt % self._constraint() def _err_msg(self, value): - return '%s is out of range (min: %s, max: %s)' % (value, - self.min, - self.max) + return _('%(value)s is out of range ' + '(min: %(min)s, max: %(max)s)') % {'value': value, + 'min': self.min, + 'max': self.max} def _is_valid(self, value, schema, context): value = Schema.str_to_num(value) @@ -432,9 +436,10 @@ class Length(Range): return fmt % self._constraint() def _err_msg(self, value): - return 'length (%d) is out of range (min: %s, max: %s)' % (len(value), - self.min, - self.max) + return _('length (%(length)d) is out of range ' + '(min: %(min)s, max: %(max)s)') % {'length': len(value), + 'min': self.min, + 'max': self.max} def _is_valid(self, value, schema, context): return super(Length, self)._is_valid(len(value), schema, context) @@ -498,8 +503,10 @@ class Modulo(Constraint): return fmt % self._constraint() def _err_msg(self, value): - return '%s is not a multiple of %s with an offset of %s)' % ( - value, self.step, self.offset) + return _('%(value)s is not a multiple of %(step)s ' + 'with an offset of %(offset)s') % {'value': value, + 'step': self.step, + 'offset': self.offset} def _is_valid(self, value, schema, context): value = Schema.str_to_num(value) @@ -542,12 +549,14 @@ class AllowedValues(Constraint): self.allowed = tuple(allowed) def _str(self): - allowed = ', '.join(str(a) for a in self.allowed) + allowed = ', '.join(json.dumps(a) for a in self.allowed) return _('Allowed values: %s') % allowed def _err_msg(self, value): - allowed = '[%s]' % ', '.join(str(a) for a in self.allowed) - return '"%s" is not an allowed value %s' % (value, allowed) + allowed = '[%s]' % ', '.join(json.dumps(a) for a in self.allowed) + return _('%(value)s is not an allowed value ' + '%(allowed)s') % {'value': json.dumps(value), + 'allowed': allowed} def _is_valid(self, value, schema, context): # For list values, check if all elements of the list are contained @@ -590,7 +599,8 @@ class AllowedPattern(Constraint): return _('Value must match pattern: %s') % self.pattern def _err_msg(self, value): - return '"%s" does not match pattern "%s"' % (value, self.pattern) + return _('"%(value)s" does not match pattern ' + '"%(pattern)s"') % {'value': value, 'pattern': self.pattern} def _is_valid(self, value, schema, context): match = self.match(value) @@ -691,7 +701,7 @@ class BaseCustomConstraint(object): try: self.validate_with_client(context.clients, value_to_validate) except self.expected_exceptions as e: - self._error_message = str(e) + self._error_message = six.text_type(e) return False else: return True diff --git a/heat/tests/openstack/aodh/test_composite_alarm.py b/heat/tests/openstack/aodh/test_composite_alarm.py index 19468c8fb8..c67af5956b 100644 --- a/heat/tests/openstack/aodh/test_composite_alarm.py +++ b/heat/tests/openstack/aodh/test_composite_alarm.py @@ -134,7 +134,7 @@ class CompositeAlarmTest(common.HeatTestCase): props = test_stack.t['resources']['cps_alarm']['Properties'] props['composite_rule']['operator'] = 'invalid' res = test_stack['cps_alarm'] - error_msg = '"invalid" is not an allowed value [or, and]' + error_msg = '"invalid" is not an allowed value' exc = self.assertRaises(exception.StackValidationFailed, res.validate) diff --git a/heat/tests/openstack/glance/test_image.py b/heat/tests/openstack/glance/test_image.py index 8210fd1650..01b3a094ac 100644 --- a/heat/tests/openstack/glance/test_image.py +++ b/heat/tests/openstack/glance/test_image.py @@ -177,10 +177,8 @@ class GlanceImageTest(common.HeatTestCase): props['disk_format'] = 'incorrect_format' image.t = image.t.freeze(properties=props) image.reparse() - error_msg = ('Property error: ' - 'resources.image.properties.disk_format: ' - '"incorrect_format" is not an allowed value ' - '[ami, ari, aki, vhd, vmdk, raw, qcow2, vdi, iso]') + error_msg = ('resources.image.properties.disk_format: ' + '"incorrect_format" is not an allowed value') self._test_validate(image, error_msg) def test_miss_container_format(self): @@ -210,10 +208,8 @@ class GlanceImageTest(common.HeatTestCase): props['container_format'] = 'incorrect_format' image.t = image.t.freeze(properties=props) image.reparse() - error_msg = ('Property error: ' - 'resources.image.properties.container_format: ' - '"incorrect_format" is not an allowed value ' - '[ami, ari, aki, bare, ova, ovf]') + error_msg = ('resources.image.properties.container_format: ' + '"incorrect_format" is not an allowed value') self._test_validate(image, error_msg) def test_miss_location(self): @@ -549,8 +545,7 @@ class GlanceWebImageTest(common.HeatTestCase): image.reparse() error_msg = ('Property error: ' 'resources.image.properties.disk_format: ' - '"incorrect_format" is not an allowed value ' - '[ami, ari, aki, vhd, vmdk, raw, qcow2, vdi, iso]') + '"incorrect_format" is not an allowed value') self._test_validate(image, error_msg) def test_miss_container_format(self): @@ -582,8 +577,7 @@ class GlanceWebImageTest(common.HeatTestCase): image.reparse() error_msg = ('Property error: ' 'resources.image.properties.container_format: ' - '"incorrect_format" is not an allowed value ' - '[ami, ari, aki, bare, ova, ovf]') + '"incorrect_format" is not an allowed value') self._test_validate(image, error_msg) def test_miss_location(self): diff --git a/heat/tests/openstack/manila/test_share.py b/heat/tests/openstack/manila/test_share.py index edaa01a6f1..ff837c9d77 100644 --- a/heat/tests/openstack/manila/test_share.py +++ b/heat/tests/openstack/manila/test_share.py @@ -229,7 +229,7 @@ class ManilaShareTest(common.HeatTestCase): stack = utils.parse_stack(tmp, stack_name='access_type') self.assertRaisesRegex( exception.StackValidationFailed, - r'.* "domain" is not an allowed value \[ip, user, cert, cephx\]', + '.* "domain" is not an allowed value', stack.validate) def test_get_live_state(self): diff --git a/heat/tests/openstack/neutron/test_neutron_rbac_policy.py b/heat/tests/openstack/neutron/test_neutron_rbac_policy.py index b67c3b65f7..0ee21acf7a 100644 --- a/heat/tests/openstack/neutron/test_neutron_rbac_policy.py +++ b/heat/tests/openstack/neutron/test_neutron_rbac_policy.py @@ -73,14 +73,12 @@ class RBACPolicyTest(common.HeatTestCase): def test_validate_action_for_network(self): msg = ('Property error: resources.rbac.properties.action: ' - '"invalid" is not an allowed value ' - r'\[access_as_shared, access_as_external\]') + '"invalid" is not an allowed value') self._test_validate_invalid_action(msg) def test_validate_action_for_qos_policy(self): msg = ('Property error: resources.rbac.properties.action: ' - '"invalid" is not an allowed value ' - r'\[access_as_shared, access_as_external\]') + '"invalid" is not an allowed value') self._test_validate_invalid_action(msg, obj_type='qos_policy') # we dont support access_as_external for qos_policy msg = ('Property error: resources.rbac.properties.action: ' diff --git a/heat/tests/test_constraints.py b/heat/tests/test_constraints.py index d91a3270d3..c2f87a68d3 100644 --- a/heat/tests/test_constraints.py +++ b/heat/tests/test_constraints.py @@ -366,7 +366,7 @@ class SchemaTest(common.HeatTestCase): self.assertIsNone(schema.validate_constraints(1)) err = self.assertRaises(exception.StackValidationFailed, schema.validate_constraints, 3) - self.assertEqual('"3" is not an allowed value [1, 2, 4]', + self.assertEqual('3 is not an allowed value [1, 2, 4]', six.text_type(err)) self.assertIsNone(schema.validate_constraints('1')) err = self.assertRaises(exception.StackValidationFailed, @@ -383,12 +383,12 @@ class SchemaTest(common.HeatTestCase): self.assertIsNone(schema.validate_constraints(1)) err = self.assertRaises(exception.StackValidationFailed, schema.validate_constraints, 3) - self.assertEqual('"3" is not an allowed value [1, 2, 4]', + self.assertEqual('3 is not an allowed value ["1", "2", "4"]', six.text_type(err)) self.assertIsNone(schema.validate_constraints('1')) err = self.assertRaises(exception.StackValidationFailed, schema.validate_constraints, '3') - self.assertEqual('"3" is not an allowed value [1, 2, 4]', + self.assertEqual('"3" is not an allowed value ["1", "2", "4"]', six.text_type(err)) def test_allowed_values_numeric_float(self): @@ -408,7 +408,7 @@ class SchemaTest(common.HeatTestCase): self.assertIsNone(schema.validate_constraints(1.1)) err = self.assertRaises(exception.StackValidationFailed, schema.validate_constraints, 3.3) - self.assertEqual('"3.3" is not an allowed value [1.1, 2.2, 4.4]', + self.assertEqual('3.3 is not an allowed value [1.1, 2.2, 4.4]', six.text_type(err)) self.assertIsNone(schema.validate_constraints('1.1')) err = self.assertRaises(exception.StackValidationFailed, @@ -425,12 +425,12 @@ class SchemaTest(common.HeatTestCase): self.assertIsNone(schema.validate_constraints(1.1)) err = self.assertRaises(exception.StackValidationFailed, schema.validate_constraints, 3.3) - self.assertEqual('"3.3" is not an allowed value [1.1, 2.2, 4.4]', + self.assertEqual('3.3 is not an allowed value ["1.1", "2.2", "4.4"]', six.text_type(err)) self.assertIsNone(schema.validate_constraints('1.1')) err = self.assertRaises(exception.StackValidationFailed, schema.validate_constraints, '3.3') - self.assertEqual('"3.3" is not an allowed value [1.1, 2.2, 4.4]', + self.assertEqual('"3.3" is not an allowed value ["1.1", "2.2", "4.4"]', six.text_type(err)) def test_to_schema_type_int(self): diff --git a/heat/tests/test_validate.py b/heat/tests/test_validate.py index 7cb336a58f..94780f5db6 100644 --- a/heat/tests/test_validate.py +++ b/heat/tests/test_validate.py @@ -1757,7 +1757,7 @@ class ValidateTest(common.HeatTestCase): stack = parser.Stack(self.ctx, 'test_stack', template) err = self.assertRaises(exception.StackValidationFailed, stack.validate) - self.assertIn('"3" is not an allowed value [1, 4, 8]', + self.assertIn('3 is not an allowed value [1, 4, 8]', six.text_type(err)) def test_validate_not_allowed_values_integer_str(self): @@ -1769,7 +1769,7 @@ class ValidateTest(common.HeatTestCase): stack = parser.Stack(self.ctx, 'test_stack', template) err = self.assertRaises(exception.StackValidationFailed, stack.validate) - self.assertIn('"3" is not an allowed value [1, 4, 8]', + self.assertIn('"3" is not an allowed value ["1", "4", "8"]', six.text_type(err)) # test with size parameter provided as number @@ -1777,7 +1777,7 @@ class ValidateTest(common.HeatTestCase): stack = parser.Stack(self.ctx, 'test_stack', template) err = self.assertRaises(exception.StackValidationFailed, stack.validate) - self.assertIn('"3" is not an allowed value [1, 4, 8]', + self.assertIn('3 is not an allowed value ["1", "4", "8"]', six.text_type(err)) def test_validate_invalid_outputs(self): diff --git a/releasenotes/notes/constraints-i18n-dc8b2652b8455196.yaml b/releasenotes/notes/constraints-i18n-dc8b2652b8455196.yaml new file mode 100644 index 0000000000..b2249752b2 --- /dev/null +++ b/releasenotes/notes/constraints-i18n-dc8b2652b8455196.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Non-ASCII text that appears in parameter constraints (e.g. in the + description of a constraint, or a list of allowed values) will now be + handled correctly when generating error messages if the constraint is not + met.