From b9923e645c3f70b1b8c7b89394b69dae9ac21326 Mon Sep 17 00:00:00 2001 From: Sergey Kraynev Date: Wed, 27 Jan 2016 06:36:52 -0500 Subject: [PATCH] Stop using Hidden property for RandomString Sequence property was hidden. According to translation rule it now changed on self.CHARACTER_CLASSES_CLASS property. As result code in validation and handle_create methods can be removed. Also unittests was changed to demostrate changes applied for Hidden property. Class with TestGenerateRandomString was changed to use new algorithm for generation random strings. Change-Id: I3f81df94b24e0f751d2070dc314f15ceb95164ce --- .../resources/openstack/heat/random_string.py | 35 ++---- .../openstack/heat/test_random_string.py | 107 +++++++++++------- 2 files changed, 75 insertions(+), 67 deletions(-) diff --git a/heat/engine/resources/openstack/heat/random_string.py b/heat/engine/resources/openstack/heat/random_string.py index 60e41cb561..d4758531ec 100644 --- a/heat/engine/resources/openstack/heat/random_string.py +++ b/heat/engine/resources/openstack/heat/random_string.py @@ -124,7 +124,11 @@ class RandomString(resource.Resource): ] ) } - ) + ), + # add defaults for backward compatibility + default=[{CHARACTER_CLASSES_CLASS: 'lettersdigits', + CHARACTER_CLASSES_MIN: 1}] + ), CHARACTER_SEQUENCES: properties.Schema( properties.Schema.LIST, @@ -196,11 +200,6 @@ class RandomString(resource.Resource): ) ] - @staticmethod - def _deprecated_random_string(sequence, length): - rand = random.SystemRandom() - return ''.join(rand.choice(sequence) for x in six.moves.xrange(length)) - def _generate_random_string(self, char_sequences, char_classes, length): random_string = "" @@ -256,18 +255,9 @@ class RandomString(resource.Resource): def validate(self): super(RandomString, self).validate() - sequence = self.properties[self.SEQUENCE] char_sequences = self.properties[self.CHARACTER_SEQUENCES] char_classes = self.properties[self.CHARACTER_CLASSES] - if sequence and (char_sequences or char_classes): - msg = (_("Cannot use deprecated '%(seq)s' property along with " - "'%(char_seqs)s' or '%(char_classes)s' properties") - % {'seq': self.SEQUENCE, - 'char_seqs': self.CHARACTER_SEQUENCES, - 'char_classes': self.CHARACTER_CLASSES}) - raise exception.StackValidationFailed(message=msg) - def char_min(char_dicts, min_prop): if char_dicts: return sum(char_dict[min_prop] for char_dict in char_dicts) @@ -286,18 +276,9 @@ class RandomString(resource.Resource): char_classes = self.properties[self.CHARACTER_CLASSES] length = self.properties[self.LENGTH] - if char_sequences or char_classes: - random_string = self._generate_random_string(char_sequences, - char_classes, - length) - else: - sequence = self.properties[self.SEQUENCE] - if not sequence: # Deprecated property not provided, use a default - sequence = "lettersdigits" - - char_seq = self._sequences[sequence] - random_string = self._deprecated_random_string(char_seq, length) - + random_string = self._generate_random_string(char_sequences, + char_classes, + length) self.data_set('value', random_string, redact=True) self.resource_id_set(self.physical_resource_name()) diff --git a/heat/tests/openstack/heat/test_random_string.py b/heat/tests/openstack/heat/test_random_string.py index 31fca7fb52..84070f2666 100644 --- a/heat/tests/openstack/heat/test_random_string.py +++ b/heat/tests/openstack/heat/test_random_string.py @@ -19,7 +19,6 @@ from testtools import matchers from heat.common import exception from heat.common import template_format -from heat.engine.resources.openstack.heat import random_string as rs from heat.engine import stack as parser from heat.engine import template from heat.tests import common @@ -38,11 +37,6 @@ Resources: Properties: length: 10 secret3: - Type: OS::Heat::RandomString - Properties: - length: 100 - sequence: octdigits - secret4: Type: OS::Heat::RandomString Properties: length: 32 @@ -60,7 +54,7 @@ Resources: min: 2 - sequence: '@' min: 5 - secret5: + secret4: Type: OS::Heat::RandomString Properties: length: 25 @@ -71,7 +65,7 @@ Resources: min: 1 - class: lowercase min: 20 - secret6: + secret5: Type: OS::Heat::RandomString Properties: length: 10 @@ -101,59 +95,74 @@ Resources: stack.store() return stack + def assert_min(self, pattern, string, minimum): + self.assertTrue(len(re.findall(pattern, string)) >= minimum) + def test_random_string(self): stack = self.create_stack(self.template_random_string) secret1 = stack['secret1'] - def assert_min(pattern, string, minimum): - self.assertTrue(len(re.findall(pattern, string)) >= minimum) - random_string = secret1.FnGetAtt('value') - assert_min('[a-zA-Z0-9]', random_string, 32) + self.assert_min('[a-zA-Z0-9]', random_string, 32) self.assertRaises(exception.InvalidTemplateAttribute, secret1.FnGetAtt, 'foo') self.assertEqual(secret1.FnGetRefId(), random_string) secret2 = stack['secret2'] random_string = secret2.FnGetAtt('value') - assert_min('[a-zA-Z0-9]', random_string, 10) + self.assert_min('[a-zA-Z0-9]', random_string, 10) self.assertEqual(secret2.FnGetRefId(), random_string) secret3 = stack['secret3'] random_string = secret3.FnGetAtt('value') - assert_min('[0-7]', random_string, 100) + self.assertEqual(32, len(random_string)) + self.assert_min('[0-9]', random_string, 1) + self.assert_min('[A-Z]', random_string, 1) + self.assert_min('[a-z]', random_string, 20) + self.assert_min('[(),\[\]{}]', random_string, 1) + self.assert_min('[$_]', random_string, 2) + self.assert_min('@', random_string, 5) self.assertEqual(secret3.FnGetRefId(), random_string) secret4 = stack['secret4'] random_string = secret4.FnGetAtt('value') - self.assertEqual(32, len(random_string)) - assert_min('[0-9]', random_string, 1) - assert_min('[A-Z]', random_string, 1) - assert_min('[a-z]', random_string, 20) - assert_min('[(),\[\]{}]', random_string, 1) - assert_min('[$_]', random_string, 2) - assert_min('@', random_string, 5) + self.assertEqual(25, len(random_string)) + self.assert_min('[0-9]', random_string, 1) + self.assert_min('[A-Z]', random_string, 1) + self.assert_min('[a-z]', random_string, 20) self.assertEqual(secret4.FnGetRefId(), random_string) secret5 = stack['secret5'] random_string = secret5.FnGetAtt('value') - self.assertEqual(25, len(random_string)) - assert_min('[0-9]', random_string, 1) - assert_min('[A-Z]', random_string, 1) - assert_min('[a-z]', random_string, 20) + self.assertEqual(10, len(random_string)) + self.assert_min('[(),\[\]{}]', random_string, 1) + self.assert_min('[$_]', random_string, 2) + self.assert_min('@', random_string, 5) self.assertEqual(secret5.FnGetRefId(), random_string) - secret6 = stack['secret6'] - random_string = secret6.FnGetAtt('value') - self.assertEqual(10, len(random_string)) - assert_min('[(),\[\]{}]', random_string, 1) - assert_min('[$_]', random_string, 2) - assert_min('@', random_string, 5) - self.assertEqual(secret6.FnGetRefId(), random_string) - # Prove the name is returned before create sets the ID - secret6.resource_id = None - self.assertEqual('secret6', secret6.FnGetRefId()) + secret5.resource_id = None + self.assertEqual('secret5', secret5.FnGetRefId()) + + def test_hidden_sequence_property(self): + hidden_prop_templ = ''' +HeatTemplateFormatVersion: '2012-12-12' +Resources: + secret: + Type: OS::Heat::RandomString + Properties: + length: 100 + sequence: octdigits + ''' + stack = self.create_stack(hidden_prop_templ) + secret = stack['secret'] + random_string = secret.FnGetAtt('value') + self.assert_min('[0-7]', random_string, 100) + self.assertEqual(secret.FnGetRefId(), random_string) + # check, that property was translated according to the TranslationRule + self.assertIsNone(secret.properties['sequence']) + expected = [{'class': u'octdigits', 'min': 1}] + self.assertEqual(expected, secret.properties['character_classes']) def test_random_string_refid_convergence_cache_data(self): t = template_format.parse(self.template_random_string) @@ -220,7 +229,6 @@ Resources: class TestGenerateRandomString(common.HeatTestCase): - scenarios = [ ('lettersdigits', dict( length=1, seq='lettersdigits', pattern='[a-zA-Z0-9]')), @@ -238,13 +246,32 @@ class TestGenerateRandomString(common.HeatTestCase): length=32, seq='octdigits', pattern='[0-7]')) ] - def test_generate_random_string(self): + template_rs = ''' +HeatTemplateFormatVersion: '2012-12-12' +Resources: + secret: + Type: OS::Heat::RandomString +''' + + def parse_stack(self, t): + stack_name = 'test_stack' + tmpl = template.Template(t) + stack = parser.Stack(utils.dummy_context(), stack_name, tmpl) + stack.validate() + stack.store() + return stack + + # test was saved to test backward compatibility with old behavior + def test_generate_random_string_backward_compatiable(self): + stack = self.parse_stack(template_format.parse(self.template_rs)) + secret = stack['secret'] + char_classes = secret.properties['character_classes'] + for char_cl in char_classes: + char_cl['class'] = self.seq # run each test multiple times to confirm random generator # doesn't generate a matching pattern by chance for i in range(1, 32): - sequence = rs.RandomString._sequences[self.seq] - r = rs.RandomString._deprecated_random_string(sequence, - self.length) + r = secret._generate_random_string(None, char_classes, self.length) self.assertThat(r, matchers.HasLength(self.length)) regex = '%s{%s}' % (self.pattern, self.length)