From d995aebb498ecf0c9faa9c9640b225db1bda0e75 Mon Sep 17 00:00:00 2001 From: Oleksii Chuprykov Date: Wed, 6 Jul 2016 13:38:58 +0300 Subject: [PATCH] Fix empty values validation in nova server In some places in validation code we assume '' value as undefined. Howewer, '' means that property is assigned, but not yet resolved, i.e. get_attr or get_resource function is used and resolved to None and next in the code, when we get the value of specific property, we convert None to default value. In this case to empty string. Closes-Bug: #1591098 Change-Id: I03a609ce87944c8efc9ec76a7a314d2de27d16d7 --- .../engine/resources/openstack/nova/server.py | 7 ++--- heat/tests/openstack/nova/test_server.py | 27 ++++++++++++++++++- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/heat/engine/resources/openstack/nova/server.py b/heat/engine/resources/openstack/nova/server.py index ae2c11424a..a56489b260 100644 --- a/heat/engine/resources/openstack/nova/server.py +++ b/heat/engine/resources/openstack/nova/server.py @@ -1344,7 +1344,8 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin, 'swap_size must be specified.') raise exception.StackValidationFailed(message=msg) - if any((volume_id, snapshot_id, image_id)): + if any((volume_id is not None, snapshot_id is not None, + image_id is not None)): bootable_vol = True return bootable_vol @@ -1399,7 +1400,7 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin, # make sure the image exists if specified. image = self.properties[self.IMAGE] - if not image and not bootable_vol: + if image is None and not bootable_vol: msg = _('Neither image nor bootable volume is specified for' ' instance %s') % self.name raise exception.StackValidationFailed(message=msg) @@ -1415,7 +1416,7 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin, networks_with_port = False for network in networks: networks_with_port = (networks_with_port or - network.get(self.NETWORK_PORT)) + network.get(self.NETWORK_PORT) is not None) self._validate_network(network) # retrieve provider's absolute limits if it will be needed diff --git a/heat/tests/openstack/nova/test_server.py b/heat/tests/openstack/nova/test_server.py index 90781a07de..8974c1568b 100644 --- a/heat/tests/openstack/nova/test_server.py +++ b/heat/tests/openstack/nova/test_server.py @@ -1062,6 +1062,10 @@ class ServersTest(common.HeatTestCase): 'instance server_with_bootable_volume', six.text_type(ex)) + web_server['Properties']['image'] = '' + server = create_server('vdb') + self.assertIsNone(server.validate()) + def test_server_validate_with_nova_keypair_resource(self): stack_name = 'srv_val_test' nova_keypair_template = ''' @@ -1357,7 +1361,7 @@ class ServersTest(common.HeatTestCase): (tmpl, stack) = self._setup_test_stack(stack_name) tmpl['Resources']['WebServer']['Properties']['networks'] = [ - {'port': 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'}] + {'port': ''}] tmpl['Resources']['WebServer']['Properties'][ 'security_groups'] = ['my_security_group'] self.patchobject(nova.NovaClientPlugin, '_create', @@ -2687,6 +2691,27 @@ class ServersTest(common.HeatTestCase): self.stub_VolumeConstraint_validate() self.assertIsNone(server.validate()) + @mock.patch.object(nova.NovaClientPlugin, '_create') + def test_validate_bdm_v2_with_unresolved_volume(self, mock_create): + stack_name = 'v2_properties' + (tmpl, stack) = self._setup_test_stack(stack_name) + del tmpl['Resources']['WebServer']['Properties']['image'] + + # empty string indicates that volume is unresolved + bdm_v2 = [{'volume_id': ''}] + wsp = tmpl.t['Resources']['WebServer']['Properties'] + wsp['block_device_mapping_v2'] = bdm_v2 + + resource_defns = tmpl.resource_definitions(stack) + server = servers.Server('server_create_image_err', + resource_defns['WebServer'], stack) + self.patchobject(nova.NovaClientPlugin, 'get_flavor', + return_value=self.mock_flavor) + self.patchobject(glance.GlanceClientPlugin, 'get_image', + return_value=self.mock_image) + self.stub_VolumeConstraint_validate() + self.assertIsNone(server.validate()) + @mock.patch.object(nova.NovaClientPlugin, '_create') def test_validate_bdm_v2_properties_no_bootable_vol(self, mock_create): stack_name = 'v2_properties'