Merge "Glance GET /v2/images fails with 500 due to erroneous policy check" into milestone-proposed
Jenkins authored 10 years ago
Gerrit Code Review committed 10 years ago
| 123 | 123 | image_repo = self.gateway.get_repo(req.context) |
| 124 | 124 | try: |
| 125 | 125 | image = image_repo.get(image_id) |
| 126 | if not image.locations: | |
| 127 | reason = _("No image data could be found") | |
| 128 | raise exception.NotFound(reason) | |
| 126 | 129 | except exception.NotFound as e: |
| 127 | 130 | raise webob.exc.HTTPNotFound(explanation=unicode(e)) |
| 128 | 131 | except exception.Forbidden as e: |
| 129 | 132 | raise webob.exc.HTTPForbidden(explanation=unicode(e)) |
| 130 | 133 | |
| 131 | if not image.locations: | |
| 132 | reason = _("No image data could be found") | |
| 133 | raise webob.exc.HTTPNotFound(reason) | |
| 134 | 134 | return image |
| 135 | 135 | |
| 136 | 136 | |
| 148 | 148 | class ResponseSerializer(wsgi.JSONResponseSerializer): |
| 149 | 149 | def download(self, response, image): |
| 150 | 150 | response.headers['Content-Type'] = 'application/octet-stream' |
| 151 | # NOTE(markwash): filesystem store (and maybe others?) cause a problem | |
| 152 | # with the caching middleware if they are not wrapped in an iterator | |
| 153 | # very strange | |
| 154 | response.app_iter = iter(image.get_data()) | |
| 151 | try: | |
| 152 | # NOTE(markwash): filesystem store (and maybe others?) cause a | |
| 153 | # problem with the caching middleware if they are not wrapped in | |
| 154 | # an iterator very strange | |
| 155 | response.app_iter = iter(image.get_data()) | |
| 156 | except exception.Forbidden as e: | |
| 157 | raise webob.exc.HTTPForbidden(unicode(e)) | |
| 155 | 158 | #NOTE(saschpe): "response.app_iter = ..." currently resets Content-MD5 |
| 156 | 159 | # (https://github.com/Pylons/webob/issues/86), so it should be set |
| 157 | 160 | # afterwards for the time being. |
| 549 | 549 | return base_href |
| 550 | 550 | |
| 551 | 551 | def _format_image(self, image): |
| 552 | image_view = dict(image.extra_properties) | |
| 553 | attributes = ['name', 'disk_format', 'container_format', 'visibility', | |
| 554 | 'size', 'status', 'checksum', 'protected', | |
| 555 | 'min_ram', 'min_disk'] | |
| 556 | for key in attributes: | |
| 557 | image_view[key] = getattr(image, key) | |
| 558 | image_view['id'] = image.image_id | |
| 559 | image_view['created_at'] = timeutils.isotime(image.created_at) | |
| 560 | image_view['updated_at'] = timeutils.isotime(image.updated_at) | |
| 561 | if image.locations: | |
| 552 | image_view = dict() | |
| 553 | try: | |
| 554 | image_view = dict(image.extra_properties) | |
| 555 | attributes = ['name', 'disk_format', 'container_format', | |
| 556 | 'visibility', 'size', 'status', 'checksum', | |
| 557 | 'protected', 'min_ram', 'min_disk'] | |
| 558 | for key in attributes: | |
| 559 | image_view[key] = getattr(image, key) | |
| 560 | image_view['id'] = image.image_id | |
| 561 | image_view['created_at'] = timeutils.isotime(image.created_at) | |
| 562 | image_view['updated_at'] = timeutils.isotime(image.updated_at) | |
| 563 | ||
| 562 | 564 | if CONF.show_multiple_locations: |
| 563 | image_view['locations'] = list(image.locations) | |
| 564 | if CONF.show_image_direct_url: | |
| 565 | if image.locations: | |
| 566 | image_view['locations'] = list(image.locations) | |
| 567 | else: | |
| 568 | # NOTE (flwang): We will still show "locations": [] if | |
| 569 | # image.locations is None to indicate it's allowed to show | |
| 570 | # locations but it's just non-existent. | |
| 571 | image_view['locations'] = [] | |
| 572 | ||
| 573 | if CONF.show_image_direct_url and image.locations: | |
| 565 | 574 | image_view['direct_url'] = image.locations[0]['url'] |
| 566 | image_view['tags'] = list(image.tags) | |
| 567 | image_view['self'] = self._get_image_href(image) | |
| 568 | image_view['file'] = self._get_image_href(image, 'file') | |
| 569 | image_view['schema'] = '/v2/schemas/image' | |
| 570 | image_view = self.schema.filter(image_view) # domain | |
| 575 | ||
| 576 | image_view['tags'] = list(image.tags) | |
| 577 | image_view['self'] = self._get_image_href(image) | |
| 578 | image_view['file'] = self._get_image_href(image, 'file') | |
| 579 | image_view['schema'] = '/v2/schemas/image' | |
| 580 | image_view = self.schema.filter(image_view) # domain | |
| 581 | except exception.Forbidden as e: | |
| 582 | raise webob.exc.HTTPForbidden(unicode(e)) | |
| 571 | 583 | return image_view |
| 572 | 584 | |
| 573 | 585 | def create(self, response, image): |
| 611 | 611 | self.dispatch(self.serializer, action, response, action_result) |
| 612 | 612 | return response |
| 613 | 613 | |
| 614 | except webob.exc.HTTPException as e: | |
| 615 | return e | |
| 614 | 616 | # return unserializable result (typically a webob exc) |
| 615 | 617 | except Exception: |
| 616 | 618 | return action_result |
| 905 | 905 | response = requests.get(path, headers=headers) |
| 906 | 906 | self.assertEqual(200, response.status_code) |
| 907 | 907 | image = json.loads(response.text) |
| 908 | self.assertFalse('locations' in image) | |
| 908 | self.assertTrue('locations' in image) | |
| 909 | self.assertTrue(image["locations"] == []) | |
| 909 | 910 | |
| 910 | 911 | # Upload some image data, setting the image location |
| 911 | 912 | path = self._url('/v2/images/%s/file' % image_id) |
| 150 | 150 | self.assertRaises(AttributeError, resource.dispatch, Controller(), |
| 151 | 151 | 'index', 'on', pants='off') |
| 152 | 152 | |
| 153 | def test_call(self): | |
| 154 | class FakeController(object): | |
| 155 | def index(self, shirt, pants=None): | |
| 156 | return (shirt, pants) | |
| 157 | ||
| 158 | resource = wsgi.Resource(FakeController(), None, None) | |
| 159 | ||
| 160 | def dispatch(self, obj, action, *args, **kwargs): | |
| 161 | if isinstance(obj, wsgi.JSONRequestDeserializer): | |
| 162 | return [] | |
| 163 | if isinstance(obj, wsgi.JSONResponseSerializer): | |
| 164 | raise webob.exc.HTTPForbidden() | |
| 165 | ||
| 166 | self.stubs.Set(wsgi.Resource, 'dispatch', dispatch) | |
| 167 | ||
| 168 | request = wsgi.Request.blank('/') | |
| 169 | ||
| 170 | response = resource.__call__(request) | |
| 171 | ||
| 172 | self.assertIsInstance(response, webob.exc.HTTPForbidden) | |
| 173 | self.assertEqual(response.status_code, 403) | |
| 174 | ||
| 153 | 175 | |
| 154 | 176 | class JSONResponseSerializerTest(test_utils.BaseTestCase): |
| 155 | 177 |
| 119 | 119 | def test_download_forbidden(self): |
| 120 | 120 | request = unit_test_utils.get_fake_request() |
| 121 | 121 | self.image_repo.result = exception.Forbidden() |
| 122 | self.assertRaises(webob.exc.HTTPForbidden, self.controller.download, | |
| 123 | request, uuidutils.generate_uuid()) | |
| 124 | ||
| 125 | def test_download_get_image_location_forbidden(self): | |
| 126 | class ImageLocations(object): | |
| 127 | def __len__(self): | |
| 128 | raise exception.Forbidden() | |
| 129 | ||
| 130 | request = unit_test_utils.get_fake_request() | |
| 131 | image = FakeImage('abcd') | |
| 132 | self.image_repo.result = image | |
| 133 | image.locations = ImageLocations() | |
| 122 | 134 | self.assertRaises(webob.exc.HTTPForbidden, self.controller.download, |
| 123 | 135 | request, uuidutils.generate_uuid()) |
| 124 | 136 | |
| 362 | 374 | self.assertEqual('application/octet-stream', |
| 363 | 375 | response.headers['Content-Type']) |
| 364 | 376 | |
| 377 | def test_download_forbidden(self): | |
| 378 | """Make sure the serializer can return 403 forbidden error instead of | |
| 379 | 500 internal server error. | |
| 380 | """ | |
| 381 | def get_data(): | |
| 382 | raise exception.Forbidden() | |
| 383 | ||
| 384 | self.stubs.Set(glance.api.policy.ImageProxy, | |
| 385 | 'get_data', | |
| 386 | get_data) | |
| 387 | request = webob.Request.blank('/') | |
| 388 | request.environ = {} | |
| 389 | response = webob.Response() | |
| 390 | response.request = request | |
| 391 | image = FakeImage(size=3, data=iter('ZZZ')) | |
| 392 | image.get_data = get_data | |
| 393 | self.assertRaises(webob.exc.HTTPForbidden, | |
| 394 | self.serializer.download, | |
| 395 | response, image) | |
| 396 | ||
| 365 | 397 | def test_upload(self): |
| 366 | 398 | request = webob.Request.blank('/') |
| 367 | 399 | request.environ = {} |
| 20 | 20 | import webob |
| 21 | 21 | |
| 22 | 22 | import glance.api.v2.images |
| 23 | from glance.common import exception | |
| 23 | 24 | from glance.openstack.common import uuidutils |
| 24 | 25 | import glance.schema |
| 25 | 26 | import glance.store |
| 1946 | 1947 | expect_next = '/v2/images?sort_key=id&sort_dir=asc&limit=10&marker=%s' |
| 1947 | 1948 | self.assertEqual(expect_next % UUID2, output['next']) |
| 1948 | 1949 | |
| 1950 | def test_index_forbidden_get_image_location(self): | |
| 1951 | """Make sure the serializer works fine no mater if current user is | |
| 1952 | authorized to get image location if the show_multiple_locations is | |
| 1953 | False. | |
| 1954 | """ | |
| 1955 | class ImageLocations(object): | |
| 1956 | def __len__(self): | |
| 1957 | raise exception.Forbidden() | |
| 1958 | ||
| 1959 | self.config(show_multiple_locations=False) | |
| 1960 | self.config(show_image_direct_url=False) | |
| 1961 | url = '/v2/images?limit=10&sort_key=id&sort_dir=asc' | |
| 1962 | request = webob.Request.blank(url) | |
| 1963 | response = webob.Response(request=request) | |
| 1964 | result = {'images': self.fixtures} | |
| 1965 | self.assertEquals(response.status_int, 200) | |
| 1966 | ||
| 1967 | # The image index should work though the user is forbidden | |
| 1968 | result['images'][0].locations = ImageLocations() | |
| 1969 | self.serializer.index(response, result) | |
| 1970 | self.assertEquals(response.status_int, 200) | |
| 1971 | ||
| 1949 | 1972 | def test_show_full_fixture(self): |
| 1950 | 1973 | expected = { |
| 1951 | 1974 | 'id': UUID1, |