From ccea7b68567afb2d5bb825d0cabf247a9526bd93 Mon Sep 17 00:00:00 2001 From: Flavio Percoco Date: Tue, 9 Jun 2015 13:11:36 +0200 Subject: [PATCH] Return empty str for permissive, none, properties In Kilo, a patch landed to allow glance for returning None values. While this was not an issue for the base and registered properties in the `schema-image.json` file, it did break backwards compatibility for the custom images created in V1 that have a vulue equal to None. In order to restore compatibility, this changes returns empty strings for those custom properties that exist in the database but that are not part of the V2 schema. This strategy will help migrating such properties when updates to the image data happens and it'll be a noop for just reads. We can't use a schema migration because we don't know off hand which of the properties in the database are the ones that would need to be migrated. While we could skip this properties entirely, this patch prefers to send the empty string back as a way to create awareness on the fact that there's an empty property in the database. Since we didn't use to return these properties, we can assume they weren't being used. Closes-bug: #1419823 Change-Id: I59bb27a892fe9485fc98a612ca0148a84123f5a2 --- glance/schema.py | 16 ++++++++++++ glance/tests/functional/__init__.py | 3 +++ glance/tests/functional/v2/test_images.py | 31 +++++++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/glance/schema.py b/glance/schema.py index 2dd74516ea..6f6fb21bd9 100644 --- a/glance/schema.py +++ b/glance/schema.py @@ -47,6 +47,22 @@ class Schema(object): for key, value in six.iteritems(obj): if self._filter_func(self.properties, key): filtered[key] = value + + # NOTE(flaper87): This exists to allow for v1, null properties, + # to be used with the V2 API. During Kilo, it was allowed for the + # later to return None values without considering that V1 allowed + # for custom properties to be None, which is something V2 doesn't + # allow for. This small hack here will set V1 custom `None` pro- + # perties to an empty string so that they will be updated along + # with the image (if an update happens). + # + # We could skip the properties that are `None` but that would bring + # back the behavior we moved away from. Note that we can't consider + # doing a schema migration because we don't know which properties + # are "custom" and which came from `schema-image` if those custom + # properties were created with v1. + if key not in self.properties and value is None: + filtered[key] = '' return filtered @staticmethod diff --git a/glance/tests/functional/__init__.py b/glance/tests/functional/__init__.py index 04643745b4..70ad44ffab 100644 --- a/glance/tests/functional/__init__.py +++ b/glance/tests/functional/__init__.py @@ -85,6 +85,7 @@ class Server(object): self.server_module = None self.stop_kill = False self.use_user_token = False + self.send_identity_credentials = False def write_conf(self, **kwargs): """ @@ -322,6 +323,7 @@ metadata_encryption_key = %(metadata_encryption_key)s registry_host = 127.0.0.1 registry_port = %(registry_port)s use_user_token = %(use_user_token)s +send_identity_credentials = %(send_identity_credentials)s log_file = %(log_file)s image_size_cap = %(image_size_cap)d delayed_delete = %(delayed_delete)s @@ -346,6 +348,7 @@ image_property_quota=%(image_property_quota)s image_tag_quota=%(image_tag_quota)s image_location_quota=%(image_location_quota)s location_strategy=%(location_strategy)s +allow_additional_image_properties = True [oslo_policy] policy_file = %(policy_file)s policy_default_rule = %(policy_default_rule)s diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py index c134594acc..124a91fd32 100644 --- a/glance/tests/functional/v2/test_images.py +++ b/glance/tests/functional/v2/test_images.py @@ -68,6 +68,37 @@ class TestImages(functional.FunctionalTest): base_headers.update(custom_headers or {}) return base_headers + def test_v1_none_properties_v2(self): + self.api_server.deployment_flavor = 'noauth' + self.api_server.use_user_token = True + self.api_server.send_identity_credentials = True + self.registry_server.deployment_flavor = '' + # Image list should be empty + self.start_servers(**self.__dict__.copy()) + + # Create an image (with two deployer-defined properties) + path = self._url('/v1/images') + headers = self._headers({'content-type': 'application/octet-stream'}) + headers.update(test_utils.minimal_headers('image-1')) + # NOTE(flaper87): Sending empty string, the server will use None + headers['x-image-meta-property-my_empty_prop'] = '' + + response = requests.post(path, headers=headers) + self.assertEqual(201, response.status_code) + data = jsonutils.loads(response.text) + image_id = data['image']['id'] + + # NOTE(flaper87): Get the image using V2 and verify + # the returned value for `my_empty_prop` is an empty + # string. + path = self._url('/v2/images/%s' % image_id) + response = requests.get(path, headers=self._headers()) + self.assertEqual(200, response.status_code) + image = jsonutils.loads(response.text) + self.assertEqual('', image['my_empty_prop']) + + self.stop_servers() + def test_image_lifecycle(self): # Image list should be empty self.start_servers(**self.__dict__.copy())