commit 86b862d1e6770f84ec6aa4406caebcbc8d8a268f Author: Stuart McLaren Date: Fri Jul 24 12:19:23 2015 +0000 V2: Do not validate image schema when listing Previously when listing images via v2, the first image of each batch was validated against the v2 schema. This could lead to unpredictable behaviour where a particular image which failed the schema check may or may not be the first of a batch. In some cases it also meant that operation by one user could affect the ability of another other to list images. Closes-bug: 1477910 diff --git a/glanceclient/tests/unit/v2/fixtures.py b/glanceclient/tests/unit/v2/fixtures.py new file mode 100644 index 0000000..08fc2cf --- /dev/null +++ b/glanceclient/tests/unit/v2/fixtures.py @@ -0,0 +1,281 @@ +# Copyright (c) 2015 OpenStack Foundation +# Copyright (c) 2015 Hewlett-Packard Development Company, L.P. +# All Rights Reserved. +# +# 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. + +images_fixture = { + "images": [ + { + "checksum": "9cb02fe7fcac26f8a25d6db3109063ae", + "container_format": "bare", + "created_at": "2015-07-23T16:58:50.000000", + "deleted": "false", + "deleted_at": "null", + "disk_format": "raw", + "id": "3fc2ba62-9a02-433e-b565-d493ffc69034", + "is_public": "false", + "min_disk": 0, + "min_ram": 0, + "name": "test", + "owner": "3447cea05d6947658d73791ed9e0ed9f", + "properties": { + "kernel_id": 1234, + "ramdisk_id": 5678 + }, + "protected": "false", + "size": 145, + "status": "active", + "updated_at": "2015-07-23T16:58:51.000000", + "virtual_size": "null" + } + ] +} + +schema_fixture = { + "additionalProperties": { + "type": "string" + }, + "links": [ + { + "href": "{self}", + "rel": "self" + }, + { + "href": "{file}", + "rel": "enclosure" + }, + { + "href": "{schema}", + "rel": "describedby" + } + ], + "name": "image", + "properties": { + "architecture": { + "description": "Operating system architecture as specified in " + "http://docs.openstack.org/trunk/openstack-compute" + "/admin/content/adding-images.html", + "is_base": "false", + "type": "string" + }, + "checksum": { + "description": "md5 hash of image contents. (READ-ONLY)", + "maxLength": 32, + "type": [ + "null", + "string" + ] + }, + "container_format": { + "description": "Format of the container", + "enum": [ + "null", + "ami", + "ari", + "aki", + "bare", + "ovf", + "ova" + ], + "type": [ + "null", + "string" + ] + }, + "created_at": { + "description": "Date and time of image registration (READ-ONLY)", + "type": "string" + }, + "direct_url": { + "description": "URL to access the image file kept in external " + "store (READ-ONLY)", + "type": "string" + }, + "disk_format": { + "description": "Format of the disk", + "enum": [ + "null", + "ami", + "ari", + "aki", + "vhd", + "vmdk", + "raw", + "qcow2", + "vdi", + "iso" + ], + "type": [ + "null", + "string" + ] + }, + "file": { + "description": "(READ-ONLY)", + "type": "string" + }, + "id": { + "description": "An identifier for the image", + "pattern": "^([0-9a-fA-F]){8}-([0-9a-fA-F]){4}-([0-9a-fA-F])" + "{4}-([0-9a-fA-F]){4}-([0-9a-fA-F]){12}$", + "type": "string" + }, + "instance_uuid": { + "description": "ID of instance used to create this image.", + "is_base": "false", + "type": "string" + }, + "kernel_id": { + "description": "ID of image stored in Glance that should be used " + "as the kernel when booting an AMI-style image.", + "is_base": "false", + "pattern": "^([0-9a-fA-F]){8}-([0-9a-fA-F]){4}-([0-9a-fA-F])" + "{4}-([0-9a-fA-F]){4}-([0-9a-fA-F]){12}$", + "type": [ + "null", + "string" + ] + }, + "locations": { + "description": "A set of URLs to access the image file kept " + "in external store", + "items": { + "properties": { + "metadata": { + "type": "object" + }, + "url": { + "maxLength": 255, + "type": "string" + } + }, + "required": [ + "url", + "metadata" + ], + "type": "object" + }, + "type": "array" + }, + "min_disk": { + "description": "Amount of disk space (in GB) required to " + "boot image.", + "type": "integer" + }, + "min_ram": { + "description": "Amount of ram (in MB) required to boot image.", + "type": "integer" + }, + "name": { + "description": "Descriptive name for the image", + "maxLength": 255, + "type": [ + "null", + "string" + ] + }, + "os_distro": { + "description": "Common name of operating system distribution as " + "specified in http://docs.openstack.org/trunk/" + "openstack-compute/admin/content/" + "adding-images.html", + "is_base": "false", + "type": "string" + }, + "os_version": { + "description": "Operating system version as specified " + "by the distributor", + "is_base": "false", + "type": "string" + }, + "owner": { + "description": "Owner of the image", + "maxLength": 255, + "type": [ + "null", + "string" + ] + }, + "protected": { + "description": "If true, image will not be deletable.", + "type": "boolean" + }, + "ramdisk_id": { + "description": "ID of image stored in Glance that should be used " + "as the ramdisk when booting an AMI-style image.", + "is_base": "false", + "pattern": "^([0-9a-fA-F]){8}-([0-9a-fA-F]){4}-([0-9a-fA-F])" + "{4}-([0-9a-fA-F]){4}-([0-9a-fA-F]){12}$", + "type": [ + "null", + "string" + ] + }, + "schema": { + "description": "(READ-ONLY)", + "type": "string" + }, + "self": { + "description": "(READ-ONLY)", + "type": "string" + }, + "size": { + "description": "Size of image file in bytes (READ-ONLY)", + "type": [ + "null", + "integer" + ] + }, + "status": { + "description": "Status of the image (READ-ONLY)", + "enum": [ + "queued", + "saving", + "active", + "killed", + "deleted", + "pending_delete" + ], + "type": "string" + }, + "tags": { + "description": "List of strings related to the image", + "items": { + "maxLength": 255, + "type": "string" + }, + "type": "array" + }, + "updated_at": { + "description": "Date and time of the last image " + "modification (READ-ONLY)", + "type": "string" + }, + "virtual_size": { + "description": "Virtual size of image in bytes (READ-ONLY)", + "type": [ + "null", + "integer" + ] + }, + "visibility": { + "description": "Scope of image accessibility", + "enum": [ + "public", + "private" + ], + "type": "string" + } + } +} diff --git a/glanceclient/tests/unit/v2/test_client_requests.py b/glanceclient/tests/unit/v2/test_client_requests.py new file mode 100644 index 0000000..44a1649 --- /dev/null +++ b/glanceclient/tests/unit/v2/test_client_requests.py @@ -0,0 +1,38 @@ +# Copyright 2013 OpenStack Foundation +# Copyright (C) 2013 Yahoo! Inc. +# All Rights Reserved. +# +# 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. + +from requests_mock.contrib import fixture as rm_fixture + +from glanceclient import client +from glanceclient.tests.unit.v2.fixtures import images_fixture +from glanceclient.tests.unit.v2.fixtures import schema_fixture +from glanceclient.tests import utils as testutils + + +class ClientTestRequests(testutils.TestCase): + def test_list_bad_image_schema(self): + # if kernel_id or ramdisk_id are not uuids, verify we can + # still perform an image listing. Regression test for bug + # 1477910 + self.requests = self.useFixture(rm_fixture.Fixture()) + self.requests.get('http://example.com/v2/schemas/image', + json=schema_fixture) + self.requests.get('http://example.com/v2/images?limit=20', + json=images_fixture) + gc = client.Client(2.2, "http://example.com/v2.1") + images = gc.images.list() + for image in images: + pass diff --git a/glanceclient/v2/images.py b/glanceclient/v2/images.py index 0cbd0d5..eac95d6 100644 --- a/glanceclient/v2/images.py +++ b/glanceclient/v2/images.py @@ -39,7 +39,18 @@ class Controller(object): @utils.memoized_property def model(self): schema = self.schema_client.get('image') - return warlock.model_factory(schema.raw(), schemas.SchemaBasedModel) + warlock_model = warlock.model_factory(schema.raw(), + schemas.SchemaBasedModel) + return warlock_model + + @utils.memoized_property + def unvalidated_model(self): + """A model which does not validate the image against the v2 schema""" + schema = self.schema_client.get('image') + warlock_model = warlock.model_factory(schema.raw(), + schemas.SchemaBasedModel) + warlock_model.validate = lambda *args, **kwargs: None + return warlock_model @staticmethod def _wrap(value): @@ -78,9 +89,6 @@ class Controller(object): :returns: generator over list of Images. """ - ori_validate_fun = self.model.validate - empty_fun = lambda *args, **kwargs: None - limit = kwargs.get('limit') # NOTE(flaper87): Don't use `get('page_size', DEFAULT_SIZE)` otherwise, # it could be possible to send invalid data to the server by passing @@ -103,21 +111,15 @@ class Controller(object): # an elegant way to pass it into the model constructor # without conflict. image.pop('self', None) - yield self.model(**image) - # NOTE(zhiyan): In order to resolve the performance issue - # of JSON schema validation for image listing case, we - # don't validate each image entry but do it only on first - # image entry for each page. - self.model.validate = empty_fun - + # We do not validate the model when listing. + # This prevents side-effects of injecting invalid + # schema values via v1. + yield self.unvalidated_model(**image) if limit: limit -= 1 if limit <= 0: raise StopIteration - # NOTE(zhiyan); Reset validation function. - self.model.validate = ori_validate_fun - try: next_url = body['next'] except KeyError: