From 7e65af5f135a272eed975edd0a797c3bdf086b82 Mon Sep 17 00:00:00 2001 From: Timur Sufiev Date: Thu, 19 May 2016 18:15:24 +0300 Subject: [PATCH] Embed support for external data sinks into api.glance In case 'data' image attribute is a base string (instead of in-memory or on-disk file), api.glance sends back an image wrapper with a redirect url and a token to its caller, so the caller could upload the file to that url directly. Provide a unit test for api.glance behavior when an external upload location is used. That also requires to fix glance stub endpoint data in keystone_data.py since it didn't reflect the reality. Also document the new HORIZON_IMAGES_UPLOAD_MODE setting that will govern direct images upload and the define approach to deprecating the old HORIZON_IMAGES_ALLOW_UPLOAD setting. The old setting is deprecated as of Newton release and planned to be removed in P. 'Removing' means that it will no longer be used / referenced at all in code, not the actual presence in settings.py (it is removed from settings.py in this commit). What really matters is if the customized value of HORIZON_IMAGES_ALLOW_UPLOAD in local_settings.py will be still considered during the deprecation period. Help text in Django Create Image form in case if local file upload was enabled was wrong, fixed that. Related-Bug: #1403129 Partially implements blueprint: horizon-glance-large-image-upload Change-Id: I24ff55e0135514fae89c20175cf9c764e871969b --- doc/source/topics/settings.rst | 45 ++++++++++++++++++- openstack_dashboard/api/glance.py | 41 +++++++++++++++-- .../dashboards/project/images/images/forms.py | 4 +- .../dashboards/project/images/images/views.py | 6 +++ .../templates/images/images/_create.html | 4 +- .../local/local_settings.py.example | 6 +++ openstack_dashboard/settings.py | 18 +++++--- .../test/api_tests/base_tests.py | 4 +- .../test/api_tests/glance_tests.py | 18 ++++++++ openstack_dashboard/test/settings.py | 9 ++-- .../test/test_data/keystone_data.py | 6 +-- 11 files changed, 138 insertions(+), 23 deletions(-) diff --git a/doc/source/topics/settings.rst b/doc/source/topics/settings.rst index 61d5094bde..9cbb63595e 100755 --- a/doc/source/topics/settings.rst +++ b/doc/source/topics/settings.rst @@ -866,23 +866,66 @@ appear on image detail pages. ``HORIZON_IMAGES_ALLOW_UPLOAD`` --------------------------------- +------------------------------- .. versionadded:: 2013.1(Grizzly) Default: ``True`` +(Deprecated) + If set to ``False``, this setting disables *local* uploads to prevent filling up the disk on the dashboard server since uploads to the Glance image store service tend to be particularly large - in the order of hundreds of megabytes to multiple gigabytes. +The setting is marked as deprecated and will be removed in P or later release. +It is superseded by the setting HORIZON_IMAGES_UPLOAD_MODE. Until the removal +the ``False`` value of HORIZON_IMAGES_ALLOW_UPLOAD overrides the value of +HORIZON_IMAGES_UPLOAD_MODE. + .. note:: This will not disable image creation altogether, as this setting does not affect images created by specifying an image location (URL) as the image source. +``HORIZON_IMAGES_UPLOAD_MODE`` +------------------------------ + +.. versionadded:: 10.0.0(Newton) + +Default: ``"legacy"`` + +Valid values are ``"direct"``, ``"legacy"`` (default) and ``"off"``. ``"off"`` +disables the ability to upload images via Horizon. It is equivalent to setting +``False`` on the deprecated setting ``HORIZON_IMAGES_ALLOW_UPLOAD``. ``legacy`` +enables local file upload by piping the image file through the Horizon's +web-server. It is equivalent to setting ``True`` on the deprecated setting +``HORIZON_IMAGES_ALLOW_UPLOAD``. ``direct`` sends the image file directly from +the web browser to Glance. This bypasses Horizon web-server which both reduces +network hops and prevents filling up Horizon web-server's filesystem. ``direct`` +is the preferred mode, but due to the following requirements it is not the default. +The ``direct`` setting requires a modern web browser, network access from the +browser to the public Glance endpoint, and CORS support to be enabled on the +Glance API service. Without CORS support, the browser will forbid the PUT request +to a location different than the Horizon server. To enable CORS support for Glance +API service, you will need to edit [cors] section of glance-api.conf file (see +`here`_ how to do it). Set `allowed_origin` to the full hostname of Horizon +web-server (e.g. http:///dashboard) and restart glance-api process. + +.. _here: http://docs.openstack.org/developer/oslo.middleware/cors.html#configuration-for-oslo-config + +.. note:: + + To maintain the compatibility with the deprecated HORIZON_IMAGES_ALLOW_UPLOAD + setting, neither ``"direct"``, nor ``"legacy"`` modes will have an effect if + HORIZON_IMAGES_ALLOW_UPLOAD is set to ``False`` - as if HORIZON_IMAGES_UPLOAD_MODE + was set to ``"off"`` itself. When HORIZON_IMAGES_ALLOW_UPLOAD is set to ``True``, + all three modes are considered, as if HORIZON_IMAGES_ALLOW_UPLOAD setting + was removed. + + ``OPENSTACK_KEYSTONE_BACKEND`` ------------------------------ diff --git a/openstack_dashboard/api/glance.py b/openstack_dashboard/api/glance.py index 8cd091a1f9..494725d0ba 100644 --- a/openstack_dashboard/api/glance.py +++ b/openstack_dashboard/api/glance.py @@ -24,14 +24,13 @@ import json import logging import os - from django.conf import settings from django.core.files.uploadedfile import InMemoryUploadedFile from django.core.files.uploadedfile import SimpleUploadedFile from django.core.files.uploadedfile import TemporaryUploadedFile - import glanceclient as glance_client +import six from six.moves import _thread as thread from horizon.utils import functions as utils @@ -205,6 +204,36 @@ def image_update(request, image_id, **kwargs): LOG.warning(msg) +def get_image_upload_mode(): + if getattr(settings, 'HORIZON_IMAGES_ALLOW_UPLOAD', None) is False: + return 'off' + mode = getattr(settings, 'HORIZON_IMAGES_UPLOAD_MODE', 'legacy') + if mode not in ('off', 'legacy', 'direct'): + LOG.warning('HORIZON_IMAGES_UPLOAD_MODE has an unrecognized value of ' + '"%s", reverting to default "legacy" value' % mode) + mode = 'legacy' + return mode + + +class ExternallyUploadedImage(base.APIResourceWrapper): + def __init__(self, apiresource, request): + self._attrs = apiresource._info.keys() + super(ExternallyUploadedImage, self).__init__(apiresource=apiresource) + image_endpoint = base.url_for(request, 'image') + # FIXME(tsufiev): Horizon doesn't work with Glance V2 API yet, + # remove hardcoded /v1 as soon as it supports both + self._url = "%s/v1/images/%s" % (image_endpoint, self.id) + self._token_id = request.user.token.id + + def to_dict(self): + base_dict = super(ExternallyUploadedImage, self).to_dict() + base_dict.update({ + 'upload_url': self._url, + 'token_id': self._token_id + }) + return base_dict + + def image_create(request, **kwargs): """Create image. @@ -226,10 +255,14 @@ def image_create(request, **kwargs): image = glanceclient(request).images.create(**kwargs) if data: - if isinstance(data, TemporaryUploadedFile): + if isinstance(data, six.string_types): + # The image data is meant to be uploaded externally, return a + # special wrapper to bypass the web server in a subsequent upload + return ExternallyUploadedImage(image, request) + elif isinstance(data, TemporaryUploadedFile): # Hack to fool Django, so we can keep file open in the new thread. data.file.close_called = True - if isinstance(data, InMemoryUploadedFile): + elif isinstance(data, InMemoryUploadedFile): # Clone a new file for InMemeoryUploadedFile. # Because the old one will be closed by Django. data = SimpleUploadedFile(data.name, diff --git a/openstack_dashboard/dashboards/project/images/images/forms.py b/openstack_dashboard/dashboards/project/images/images/forms.py index 78b1fa51fe..431cddbeb8 100644 --- a/openstack_dashboard/dashboards/project/images/images/forms.py +++ b/openstack_dashboard/dashboards/project/images/images/forms.py @@ -180,7 +180,7 @@ class CreateImageForm(forms.SelfHandlingForm): def __init__(self, request, *args, **kwargs): super(CreateImageForm, self).__init__(request, *args, **kwargs) - if (not settings.HORIZON_IMAGES_ALLOW_UPLOAD or + if (api.glance.get_image_upload_mode() == 'off' or not policy.check((("image", "upload_image"),), request)): self._hide_file_source_type() if not policy.check((("image", "set_image_location"),), request): @@ -271,7 +271,7 @@ class CreateImageForm(forms.SelfHandlingForm): meta = create_image_metadata(data) # Add image source file or URL to metadata - if (settings.HORIZON_IMAGES_ALLOW_UPLOAD and + if (api.glance.get_image_upload_mode() != 'off' and policy.check((("image", "upload_image"),), request) and data.get('image_file', None)): meta['data'] = self.files['image_file'] diff --git a/openstack_dashboard/dashboards/project/images/images/views.py b/openstack_dashboard/dashboards/project/images/images/views.py index 5c7e8c35b0..853a6ab7d5 100644 --- a/openstack_dashboard/dashboards/project/images/images/views.py +++ b/openstack_dashboard/dashboards/project/images/images/views.py @@ -67,6 +67,12 @@ class CreateView(forms.ModalFormView): initial[name] = tmp return initial + def get_context_data(self, **kwargs): + context = super(CreateView, self).get_context_data(**kwargs) + upload_mode = api.glance.get_image_upload_mode() + context['image_upload_enabled'] = upload_mode != 'off' + return context + class UpdateView(forms.ModalFormView): form_class = project_forms.UpdateImageForm diff --git a/openstack_dashboard/dashboards/project/images/templates/images/images/_create.html b/openstack_dashboard/dashboards/project/images/templates/images/images/_create.html index a85494d697..e3c471a932 100644 --- a/openstack_dashboard/dashboards/project/images/templates/images/images/_create.html +++ b/openstack_dashboard/dashboards/project/images/templates/images/images/_create.html @@ -8,7 +8,7 @@ {% block modal-body-right %}

{% trans "Description:" %}

- {% if HORIZON_IMAGES_ALLOW_UPLOAD %} + {% if image_upload_enabled %} {% trans "Images can be provided via an HTTP/HTTPS URL or be uploaded from your local file system." %} {% else %} {% trans "Currently only images available via an HTTP/HTTPS URL are supported. The image location must be accessible to the Image Service." %} @@ -16,7 +16,7 @@

{% trans "Please note: " %} - {% if HORIZON_IMAGES_ALLOW_UPLOAD %} + {% if image_upload_enabled %} {% trans "If you select an image via an HTTP/HTTPS URL, the Image Location field MUST be a valid and direct URL to the image binary; it must also be accessible to the Image Service. URLs that redirect or serve error pages will result in unusable images." %} {% else %} {% trans "The Image Location field MUST be a valid and direct URL to the image binary. URLs that redirect or serve error pages will result in unusable images." %} diff --git a/openstack_dashboard/local/local_settings.py.example b/openstack_dashboard/local/local_settings.py.example index 679d6c9edd..2d77cf74ed 100644 --- a/openstack_dashboard/local/local_settings.py.example +++ b/openstack_dashboard/local/local_settings.py.example @@ -358,6 +358,12 @@ IMAGE_CUSTOM_PROPERTY_TITLES = { # table. IMAGE_RESERVED_CUSTOM_PROPERTIES = [] +# Set to 'legacy' or 'direct' to allow users to upload images to glance via +# Horizon server. When enabled, a file form field will appear on the create +# image form. If set to 'off', there will be no file form field on the create +# image form. See documentation for deployment considerations. +#HORIZON_IMAGES_UPLOAD_MODE = 'legacy' + # OPENSTACK_ENDPOINT_TYPE specifies the endpoint type to use for the endpoints # in the Keystone service catalog. Use this setting when Horizon is running # external to the OpenStack environment. The default is 'publicURL'. diff --git a/openstack_dashboard/settings.py b/openstack_dashboard/settings.py index 666e7fe07f..682e5f8729 100644 --- a/openstack_dashboard/settings.py +++ b/openstack_dashboard/settings.py @@ -82,11 +82,6 @@ HORIZON_CONFIG = { 'integration_tests_support': INTEGRATION_TESTS_SUPPORT } -# Set to True to allow users to upload images to glance via Horizon server. -# When enabled, a file form field will appear on the create image form. -# See documentation for deployment considerations. -HORIZON_IMAGES_ALLOW_UPLOAD = True - # The OPENSTACK_IMAGE_BACKEND settings can be used to customize features # in the OpenStack Dashboard related to the Image service, such as the list # of supported image formats. @@ -434,3 +429,16 @@ HORIZON_COMPRESS_OFFLINE_CONTEXT_BASE = { if DEBUG: logging.basicConfig(level=logging.DEBUG) + + +# Here comes the Django settings deprecation section. Being at the very end +# of settings.py allows it to catch the settings defined in local_settings.py +# or inside one of local_settings.d/ snippets. +if 'HORIZON_IMAGES_ALLOW_UPLOAD' in globals(): + message = 'The setting HORIZON_IMAGES_ALLOW_UPLOAD is deprecated in ' \ + 'Newton and will be removed in P release. Use the setting ' \ + 'HORIZON_IMAGES_UPLOAD_MODE instead.' + if not HORIZON_IMAGES_ALLOW_UPLOAD: + message += ' Keep in mind that HORIZON_IMAGES_ALLOW_UPLOAD set to ' \ + 'False overrides the value of HORIZON_IMAGES_UPLOAD_MODE.' + logging.warning(message) diff --git a/openstack_dashboard/test/api_tests/base_tests.py b/openstack_dashboard/test/api_tests/base_tests.py index e707ea1bd5..7780573df3 100644 --- a/openstack_dashboard/test/api_tests/base_tests.py +++ b/openstack_dashboard/test/api_tests/base_tests.py @@ -190,10 +190,10 @@ class ApiHelperTests(test.TestCase): def test_url_for(self): url = api_base.url_for(self.request, 'image') - self.assertEqual('http://public.glance.example.com:9292/v1', url) + self.assertEqual('http://public.glance.example.com:9292', url) url = api_base.url_for(self.request, 'image', endpoint_type='adminURL') - self.assertEqual('http://admin.glance.example.com:9292/v1', url) + self.assertEqual('http://admin.glance.example.com:9292', url) url = api_base.url_for(self.request, 'compute') self.assertEqual('http://public.nova.example.com:8774/v2', url) diff --git a/openstack_dashboard/test/api_tests/glance_tests.py b/openstack_dashboard/test/api_tests/glance_tests.py index c3662378bc..6d0feef462 100644 --- a/openstack_dashboard/test/api_tests/glance_tests.py +++ b/openstack_dashboard/test/api_tests/glance_tests.py @@ -20,6 +20,7 @@ from django.conf import settings from django.test.utils import override_settings from openstack_dashboard import api +from openstack_dashboard.api import base from openstack_dashboard.test import helpers as test @@ -311,3 +312,20 @@ class GlanceApiTests(test.APITestCase): res_types = api.glance.metadefs_resource_types_list(self.request) self.assertItemsEqual(res_types, []) + + def test_image_create_external_upload(self): + expected_image = self.images.first() + service = base.get_service_from_catalog(self.service_catalog, 'image') + base_url = base.get_url_for_service(service, 'RegionOne', 'publicURL') + file_upload_url = '%s/v1/images/%s' % (base_url, expected_image.id) + + glanceclient = self.stub_glanceclient() + glanceclient.images = self.mox.CreateMockAnything() + glanceclient.images.create().AndReturn(expected_image) + self.mox.ReplayAll() + + actual_image = api.glance.image_create(self.request, data='sample.iso') + actual_image_dict = actual_image.to_dict() + self.assertEqual(file_upload_url, actual_image_dict['upload_url']) + self.assertEqual(self.request.user.token.id, + actual_image_dict['token_id']) diff --git a/openstack_dashboard/test/settings.py b/openstack_dashboard/test/settings.py index d78a5a1c7e..fdd0a4afc0 100644 --- a/openstack_dashboard/test/settings.py +++ b/openstack_dashboard/test/settings.py @@ -118,10 +118,11 @@ HORIZON_CONFIG['swift_panel'] = 'legacy' find_static_files(HORIZON_CONFIG, AVAILABLE_THEMES, THEME_COLLECTION_DIR, ROOT_PATH) -# Set to True to allow users to upload images to glance via Horizon server. -# When enabled, a file form field will appear on the create image form. -# See documentation for deployment considerations. -HORIZON_IMAGES_ALLOW_UPLOAD = True +# Set to 'legacy' or 'direct' to allow users to upload images to glance via +# Horizon server. When enabled, a file form field will appear on the create +# image form. If set to 'off', there will be no file form field on the create +# image form. See documentation for deployment considerations. +HORIZON_IMAGES_UPLOAD_MODE = 'legacy' AVAILABLE_REGIONS = [ ('http://localhost:5000/v2.0', 'local'), diff --git a/openstack_dashboard/test/test_data/keystone_data.py b/openstack_dashboard/test/test_data/keystone_data.py index a57cc55f3b..3b9cd1517a 100644 --- a/openstack_dashboard/test/test_data/keystone_data.py +++ b/openstack_dashboard/test/test_data/keystone_data.py @@ -68,9 +68,9 @@ SERVICE_CATALOG = [ "endpoints_links": [], "endpoints": [ {"region": "RegionOne", - "adminURL": "http://admin.glance.example.com:9292/v1", - "internalURL": "http://int.glance.example.com:9292/v1", - "publicURL": "http://public.glance.example.com:9292/v1"}]}, + "adminURL": "http://admin.glance.example.com:9292", + "internalURL": "http://int.glance.example.com:9292", + "publicURL": "http://public.glance.example.com:9292"}]}, {"type": "identity", "name": "keystone", "endpoints_links": [],