From 0e1279d05c3b2de7127f9e4ee752d5771514cc74 Mon Sep 17 00:00:00 2001 From: Timur Sufiev Date: Tue, 24 May 2016 14:54:54 +0300 Subject: [PATCH] [NG] Support local file upload in Create Image workflow First, now there are 2 '/api/glance/images/ API wrapper endpoints for creating a new image - POST and PUT. The POST endpoint which existed before now resides at PUT. This was done to support legacy (i.e. proxied by web-server) file uploads in Angular Create Image, because Django (which we need to use in that case) doesn't correctly process PUT request. So, if local file binary payload is added to the form contents, we send it using POST request. Second, speaking of '/api/glance/images' PUT request previously known as POST... Where before the POST call to Horizon REST API wrappers returned the final image object that Glance just created, now there are two possibilities for what happens after PUT is sent. * When Create Image form Source Type is set to URL, then everything is going as before. * When Source Type is set to File, then just a file name instead of an actual Blob is sent to '/api/glance/images', then the Glance Image is queued for creation and Horizon web-server responds with an Image object which dict() representation has 2 additional keys: 'upload_url' and 'token_id'. The 'upload_url' tells the location for the subsequent CORS request, while 'token_id' is passed as a header in that request, so Keystone would let it in. CORS upload is started immediately as Image is queued for creation (first promise is resolved) and returns the second promise, which is resolved once the upload finishes. The modal form hangs until second promise resolves to indicate that upload is in progress. Upload progress notification is added in a follow-up patch. DEPLOY NOTES The client-side code relies on CORS being enabled for Glance service (otherwise browser would forbid the PUT request to a location different from the one form content came from). In a Devstack setup you'll need to edit [cors] section of glance-api.conf file, setting `allowed_origin` setting to the full hostname of the web server (say, http:///dashboard). Related-Bug: #1467890 Implements blueprint: horizon-glance-large-image-upload Change-Id: I5d842d614c16d3250380ea1dc1c6e0289d206fb5 --- horizon/static/framework/framework.module.js | 20 +++ horizon/static/framework/util/http/http.js | 30 ++++- .../static/framework/util/http/http.spec.js | 87 +++++++++++-- openstack_dashboard/api/rest/config.py | 9 +- openstack_dashboard/api/rest/glance.py | 37 +++++- openstack_dashboard/static/app/app.module.js | 1 - .../images/actions/create.action.service.js | 5 + .../actions/create.action.service.spec.js | 80 ++++++++++++ .../create-image/create-image.controller.js | 27 +++- .../create-image.controller.spec.js | 51 +++++++- .../steps/create-image/create-image.html | 41 +++++- .../openstack-service-api/glance.service.js | 36 +++++- .../glance.service.spec.js | 120 ++++++++++++++++-- .../openstack-service-api.module.js | 2 +- .../test/api_tests/glance_rest_tests.py | 12 +- 15 files changed, 505 insertions(+), 53 deletions(-) diff --git a/horizon/static/framework/framework.module.js b/horizon/static/framework/framework.module.js index fd6ba43013..16e901a4e7 100644 --- a/horizon/static/framework/framework.module.js +++ b/horizon/static/framework/framework.module.js @@ -57,6 +57,7 @@ // if user is not authorized, log user out // this can happen when session expires $httpProvider.interceptors.push(redirect); + $httpProvider.interceptors.push(stripAjaxHeaderForCORS); redirect.$inject = ['$q']; @@ -71,6 +72,25 @@ } }; } + + stripAjaxHeaderForCORS.$inject = []; + // Standard CORS middleware used in OpenStack services doesn't expect + // X-Requested-With header to be set for requests and rejects requests + // which have it. Since there is no reason to treat Horizon specially when + // dealing handling CORS requests, it's better for Horizon not to set this + // header when it sends CORS requests. Detect CORS request by presence of + // X-Auth-Token headers which normally should be provided because of + // Keystone authentication. + function stripAjaxHeaderForCORS() { + return { + request: function(config) { + if ('X-Auth-Token' in config.headers) { + delete config.headers['X-Requested-With']; + } + return config; + } + }; + } } run.$inject = ['$window', '$rootScope']; diff --git a/horizon/static/framework/util/http/http.js b/horizon/static/framework/util/http/http.js index 54b91553be..de7b90ac62 100644 --- a/horizon/static/framework/util/http/http.js +++ b/horizon/static/framework/util/http/http.js @@ -29,11 +29,17 @@ limitations under the License. var httpCall = function (method, url, data, config) { var backend = $http; - /* eslint-disable angular/window-service */ - url = $window.WEBROOT + url; - /* eslint-enable angular/window-service */ + // An external call goes directly to some OpenStack service, say Glance + // API, not to the Horizon API wrapper layer. Thus it doesn't need a + // WEBROOT prefix + var external = pop(config, 'external'); + if (!external) { + /* eslint-disable angular/window-service */ + url = $window.WEBROOT + url; + /* eslint-enable angular/window-service */ - url = url.replace(/\/+/g, '/'); + url = url.replace(/\/+/g, '/'); + } if (angular.isUndefined(config)) { config = {}; @@ -44,7 +50,10 @@ limitations under the License. if (angular.isDefined(data)) { config.data = data; } - if (angular.isObject(config.data)) { + + if (uploadService.isFile(config.data)) { + backend = uploadService.http; + } else if (angular.isObject(config.data)) { for (var key in config.data) { if (config.data.hasOwnProperty(key) && uploadService.isFile(config.data[key])) { backend = uploadService.upload; @@ -52,7 +61,6 @@ limitations under the License. } } } - return backend(config); }; @@ -77,4 +85,14 @@ limitations under the License. return httpCall('DELETE', url, data, config); }; } + + function pop(obj, key) { + if (!angular.isObject(obj)) { + return undefined; + } + var value = obj[key]; + delete obj[key]; + return value; + } + }()); diff --git a/horizon/static/framework/util/http/http.spec.js b/horizon/static/framework/util/http/http.spec.js index de66577603..c26d0a4339 100644 --- a/horizon/static/framework/util/http/http.spec.js +++ b/horizon/static/framework/util/http/http.spec.js @@ -9,8 +9,11 @@ describe('api service', function () { var api, $httpBackend; + var WEBROOT = '/horizon/'; - beforeEach(module('horizon.framework')); + beforeEach(module('horizon.framework', function($provide) { + $provide.value('$window', {WEBROOT: WEBROOT}); + })); beforeEach(inject(function ($injector) { api = $injector.get('horizon.framework.util.http.service'); $httpBackend = $injector.get('$httpBackend'); @@ -26,10 +29,11 @@ function testGoodCall(apiMethod, verb, data) { var called = {}; + var url = WEBROOT + 'good'; data = data || 'some complicated data'; var suppliedData = verb === 'GET' ? undefined : data; - $httpBackend.when(verb, '/good', suppliedData).respond({status: 'good'}); - $httpBackend.expect(verb, '/good', suppliedData); + $httpBackend.when(verb, url, suppliedData).respond({status: 'good'}); + $httpBackend.expect(verb, url, suppliedData); apiMethod('/good', suppliedData).success(function (data) { called.data = data; }); @@ -39,9 +43,10 @@ function testBadCall(apiMethod, verb) { var called = {}; + var url = WEBROOT + 'bad'; var suppliedData = verb === 'GET' ? undefined : 'some complicated data'; - $httpBackend.when(verb, '/bad', suppliedData).respond(500, ''); - $httpBackend.expect(verb, '/bad', suppliedData); + $httpBackend.when(verb, url, suppliedData).respond(500, ''); + $httpBackend.expect(verb, url, suppliedData); apiMethod('/bad', suppliedData).error(function () { called.called = true; }); @@ -89,8 +94,24 @@ testBadCall(api.delete, 'DELETE'); }); - describe('Upload.upload() call', function () { - var Upload; + describe('WEBROOT handling', function() { + it('respects WEBROOT by default', function() { + var expectedUrl = WEBROOT + 'good'; + $httpBackend.when('GET', expectedUrl).respond(200, ''); + $httpBackend.expect('GET', expectedUrl); + api.get('/good'); + }); + + it('ignores WEBROOT with external = true flag', function() { + var expectedUrl = '/good'; + $httpBackend.when('GET', expectedUrl).respond(200, ''); + $httpBackend.expect('GET', expectedUrl); + api.get('/good', {external: true}); + }); + }); + + describe('Upload service', function () { + var Upload, file; var called = {}; beforeEach(inject(function ($injector) { @@ -98,22 +119,64 @@ spyOn(Upload, 'upload').and.callFake(function (config) { called.config = config; }); + spyOn(Upload, 'http').and.callFake(function (config) { + called.config = config; + }); + file = new File(['part'], 'filename.sample'); })); - it('is used when there is a File() blob inside data', function () { - var file = new File(['part'], 'filename.sample'); - + it('upload() is used when there is a File() blob inside data', function () { api.post('/good', {first: file, second: 'the data'}); expect(Upload.upload).toHaveBeenCalled(); expect(called.config.data).toEqual({first: file, second: 'the data'}); }); - it('is NOT used in case there are no File() blobs inside data', function() { + it('upload() is NOT used when a File() blob is passed as data', function () { + api.post('/good', file); + expect(Upload.upload).not.toHaveBeenCalled(); + }); + + it('upload() is NOT used in case there are no File() blobs inside data', function() { testGoodCall(api.post, 'POST', {second: 'the data'}); expect(Upload.upload).not.toHaveBeenCalled(); }); - }); + it('upload() respects WEBROOT by default', function() { + api.post('/good', {first: file}); + expect(called.config.url).toEqual(WEBROOT + 'good'); + }); + it('upload() ignores WEBROOT with external = true flag', function() { + api.post('/good', {first: file}, {external: true}); + expect(called.config.url).toEqual('/good'); + }); + + it('http() is used when a File() blob is passed as data', function () { + api.post('/good', file); + expect(Upload.http).toHaveBeenCalled(); + expect(called.config.data).toEqual(file); + }); + + it('http() is NOT used when there is a File() blob inside data', function () { + api.post('/good', {first: file, second: 'the data'}); + expect(Upload.http).not.toHaveBeenCalled(); + }); + + it('http() is NOT used when no File() blobs are passed at all', function() { + testGoodCall(api.post, 'POST', {second: 'the data'}); + expect(Upload.http).not.toHaveBeenCalled(); + }); + + it('http() respects WEBROOT by default', function() { + api.post('/good', file); + expect(called.config.url).toEqual(WEBROOT + 'good'); + }); + + it('http() ignores WEBROOT with external = true flag', function() { + api.post('/good', file, {external: true}); + expect(called.config.url).toEqual('/good'); + }); + + }); }); }()); diff --git a/openstack_dashboard/api/rest/config.py b/openstack_dashboard/api/rest/config.py index f612c259bb..f655add70d 100644 --- a/openstack_dashboard/api/rest/config.py +++ b/openstack_dashboard/api/rest/config.py @@ -16,6 +16,7 @@ from django.conf import settings from django.views import generic +from openstack_dashboard import api from openstack_dashboard.api.rest import urls from openstack_dashboard.api.rest import utils as rest_utils @@ -38,7 +39,13 @@ class Settings(generic.View): Examples of settings: OPENSTACK_HYPERVISOR_FEATURES """ url_regex = r'settings/$' + SPECIALS = { + 'HORIZON_IMAGES_UPLOAD_MODE': api.glance.get_image_upload_mode() + } @rest_utils.ajax() def get(self, request): - return {k: getattr(settings, k, None) for k in settings_allowed} + plain_settings = {k: getattr(settings, k, None) for k + in settings_allowed if k not in self.SPECIALS} + plain_settings.update(self.SPECIALS) + return plain_settings diff --git a/openstack_dashboard/api/rest/glance.py b/openstack_dashboard/api/rest/glance.py index 8bf2b09937..95c1a7516d 100644 --- a/openstack_dashboard/api/rest/glance.py +++ b/openstack_dashboard/api/rest/glance.py @@ -14,6 +14,8 @@ """API for the glance service. """ +from django import forms +from django.views.decorators.csrf import csrf_exempt from django.views import generic from six.moves import zip as izip @@ -115,6 +117,10 @@ class ImageProperties(generic.View): ) +class UploadObjectForm(forms.Form): + data = forms.FileField(required=False) + + @urls.register class Images(generic.View): """API for Glance images. @@ -162,8 +168,26 @@ class Images(generic.View): 'has_prev_data': has_prev_data, } - @rest_utils.ajax(data_required=True) + # note: not an AJAX request - the body will be raw file content mixed with + # metadata + @csrf_exempt def post(self, request): + form = UploadObjectForm(request.POST, request.FILES) + if not form.is_valid(): + raise rest_utils.AjaxError(500, 'Invalid request') + + data = form.clean() + meta = create_image_metadata(request.POST) + meta['data'] = data['data'] + + image = api.glance.image_create(request, **meta) + return rest_utils.CreatedResponse( + '/api/glance/images/%s' % image.name, + image.to_dict() + ) + + @rest_utils.ajax(data_required=True) + def put(self, request): """Create an Image. Create an Image using the parameters supplied in the POST @@ -193,10 +217,13 @@ class Images(generic.View): """ meta = create_image_metadata(request.DATA) - if request.DATA.get('import_data'): - meta['copy_from'] = request.DATA.get('image_url') + if request.DATA.get('image_url'): + if request.DATA.get('import_data'): + meta['copy_from'] = request.DATA.get('image_url') + else: + meta['location'] = request.DATA.get('image_url') else: - meta['location'] = request.DATA.get('image_url') + meta['data'] = request.DATA.get('data') image = api.glance.image_create(request, **meta) return rest_utils.CreatedResponse( @@ -326,7 +353,7 @@ def handle_unknown_properties(data, meta): 'container_format', 'min_disk', 'min_ram', 'name', 'properties', 'kernel', 'ramdisk', 'tags', 'import_data', 'source', - 'image_url', 'source_type', + 'image_url', 'source_type', 'data', 'checksum', 'created_at', 'deleted', 'is_copying', 'deleted_at', 'is_public', 'virtual_size', 'status', 'size', 'owner', 'id', 'updated_at'] diff --git a/openstack_dashboard/static/app/app.module.js b/openstack_dashboard/static/app/app.module.js index c376688be1..8c2df73f1b 100644 --- a/openstack_dashboard/static/app/app.module.js +++ b/openstack_dashboard/static/app/app.module.js @@ -28,7 +28,6 @@ 'ngSanitize', 'schemaForm', 'smart-table', - 'ngFileUpload', 'ui.bootstrap' ]; diff --git a/openstack_dashboard/static/app/core/images/actions/create.action.service.js b/openstack_dashboard/static/app/core/images/actions/create.action.service.js index 93ad47804b..454ff08197 100644 --- a/openstack_dashboard/static/app/core/images/actions/create.action.service.js +++ b/openstack_dashboard/static/app/core/images/actions/create.action.service.js @@ -110,6 +110,11 @@ function submit() { var finalModel = angular.extend({}, model.image, model.metadata); + if (finalModel.source_type === 'url') { + delete finalModel.data; + } else { + delete finalModel.image_url; + } return glance.createImage(finalModel).then(onCreateImage); } diff --git a/openstack_dashboard/static/app/core/images/actions/create.action.service.spec.js b/openstack_dashboard/static/app/core/images/actions/create.action.service.spec.js index f8875735ba..3e5eb37fa0 100644 --- a/openstack_dashboard/static/app/core/images/actions/create.action.service.spec.js +++ b/openstack_dashboard/static/app/core/images/actions/create.action.service.spec.js @@ -119,6 +119,86 @@ id: '2', prop1: '11', prop3: '3'}); }); + it('does not pass location to create image if source_type is NOT url', function() { + var image = {name: 'Test', source_type: 'file-direct', image_url: 'http://somewhere', + data: {name: 'test_file'} + }; + + spyOn($scope, '$emit').and.callThrough(); + spyOn(glanceAPI, 'createImage').and.callThrough(); + spyOn(wizardModalService, 'modal').and.callThrough(); + + service.initScope($scope); + service.perform(); + $scope.$emit(events.IMAGE_CHANGED, image); + + var modalArgs = wizardModalService.modal.calls.argsFor(0)[0]; + modalArgs.submit(); + + expect(glanceAPI.createImage).toHaveBeenCalledWith({ name: 'Test', + source_type: 'file-direct', data: {name: 'test_file'}}); + }); + + it('does not pass file to create image if source_type is url', function() { + var image = {name: 'Test', source_type: 'url', image_url: 'http://somewhere', + data: {name: 'test_file'} + }; + + spyOn($scope, '$emit').and.callThrough(); + spyOn(glanceAPI, 'createImage').and.callThrough(); + spyOn(wizardModalService, 'modal').and.callThrough(); + + service.initScope($scope); + service.perform(); + $scope.$emit(events.IMAGE_CHANGED, image); + + var modalArgs = wizardModalService.modal.calls.argsFor(0)[0]; + modalArgs.submit(); + + expect(glanceAPI.createImage).toHaveBeenCalledWith({ name: 'Test', + source_type: 'url', image_url: 'http://somewhere'}); + }); + + it('does not pass location to create image if source_type is NOT url', function() { + var image = {name: 'Test', source_type: 'file-direct', image_url: 'http://somewhere', + data: {name: 'test_file'} + }; + + spyOn($scope, '$emit').and.callThrough(); + spyOn(glanceAPI, 'createImage').and.callThrough(); + spyOn(wizardModalService, 'modal').and.callThrough(); + + service.initScope($scope); + service.perform(); + $scope.$emit(events.IMAGE_CHANGED, image); + + var modalArgs = wizardModalService.modal.calls.argsFor(0)[0]; + modalArgs.submit(); + + expect(glanceAPI.createImage).toHaveBeenCalledWith({ name: 'Test', + source_type: 'file-direct', data: {name: 'test_file'}}); + }); + + it('does not pass file to create image if source_type is url', function() { + var image = {name: 'Test', source_type: 'url', image_url: 'http://somewhere', + data: {name: 'test_file'} + }; + + spyOn($scope, '$emit').and.callThrough(); + spyOn(glanceAPI, 'createImage').and.callThrough(); + spyOn(wizardModalService, 'modal').and.callThrough(); + + service.initScope($scope); + service.perform(); + $scope.$emit(events.IMAGE_CHANGED, image); + + var modalArgs = wizardModalService.modal.calls.argsFor(0)[0]; + modalArgs.submit(); + + expect(glanceAPI.createImage).toHaveBeenCalledWith({ name: 'Test', + source_type: 'url', image_url: 'http://somewhere'}); + }); + it('should raise event even if update meta data fails', function() { var image = { name: 'Test', id: '2' }; var failedPromise = function() { diff --git a/openstack_dashboard/static/app/core/images/steps/create-image/create-image.controller.js b/openstack_dashboard/static/app/core/images/steps/create-image/create-image.controller.js index 01056c9394..145a26b29d 100644 --- a/openstack_dashboard/static/app/core/images/steps/create-image/create-image.controller.js +++ b/openstack_dashboard/static/app/core/images/steps/create-image/create-image.controller.js @@ -46,14 +46,16 @@ ) { var ctrl = this; - settings.getSettings().then(getConfiguredFormats); + settings.getSettings().then(getConfiguredFormatsAndModes); ctrl.validationRules = validationRules; ctrl.imageFormats = imageFormats; ctrl.diskFormats = []; + ctrl.prepareUpload = prepareUpload; ctrl.image = { source_type: 'url', image_url: '', + data: {}, is_copying: true, protected: false, min_disk: 0, @@ -73,6 +75,10 @@ { label: gettext('No'), value: false } ]; + ctrl.imageSourceOptions = [ + { label: gettext('URL'), value: 'url' } + ]; + ctrl.imageVisibilityOptions = [ { label: gettext('Public'), value: 'public'}, { label: gettext('Private'), value: 'private' } @@ -82,6 +88,7 @@ ctrl.ramdiskImages = []; ctrl.setFormats = setFormats; + ctrl.isLocalFileUpload = isLocalFileUpload; init(); @@ -93,18 +100,32 @@ /////////////////////////// - function getConfiguredFormats(response) { + function prepareUpload(file) { + ctrl.image.data = file; + } + + function getConfiguredFormatsAndModes(response) { var settingsFormats = response.OPENSTACK_IMAGE_FORMATS; + var uploadMode = response.HORIZON_IMAGES_UPLOAD_MODE; var dupe = angular.copy(imageFormats); angular.forEach(dupe, function stripUnknown(name, key) { if (settingsFormats.indexOf(key) === -1) { delete dupe[key]; } }); - + if (uploadMode !== 'off') { + ctrl.imageSourceOptions.splice(0, 0, { + label: gettext('File'), value: 'file-' + uploadMode + }); + } ctrl.imageFormats = dupe; } + function isLocalFileUpload() { + var type = ctrl.image.source_type; + return (type === 'file-legacy' || type === 'file-direct'); + } + // emits new data to parent listeners function watchImageCollection(newValue, oldValue) { if (newValue !== oldValue) { diff --git a/openstack_dashboard/static/app/core/images/steps/create-image/create-image.controller.spec.js b/openstack_dashboard/static/app/core/images/steps/create-image/create-image.controller.spec.js index 2f08d56370..49419ce3ec 100644 --- a/openstack_dashboard/static/app/core/images/steps/create-image/create-image.controller.spec.js +++ b/openstack_dashboard/static/app/core/images/steps/create-image/create-image.controller.spec.js @@ -165,7 +165,7 @@ }); }); - describe('getConfiguredFormats', function() { + describe('getConfiguredFormatsAndModes', function() { it('uses the settings for the source of allowed image formats', function() { var ctrl = createController(); @@ -178,6 +178,55 @@ }; expect(ctrl.imageFormats).toEqual(expectation); }); + + describe('upload mode', function() { + var urlSourceOption = { label: gettext('URL'), value: 'url' }; + + it('set to "off" disables local file upload', function() { + var ctrl = createController(); + settingsCall.resolve({ + OPENSTACK_IMAGE_FORMATS: [], + HORIZON_IMAGES_UPLOAD_MODE: 'off' + }); + $timeout.flush(); + expect(ctrl.imageSourceOptions).toEqual([urlSourceOption]); + }); + + it('set to a non-"off" value enables local file upload', function() { + var ctrl = createController(); + var fileSourceOption = { label: gettext('File'), value: 'file-sample' }; + settingsCall.resolve({ + OPENSTACK_IMAGE_FORMATS: [], + HORIZON_IMAGES_UPLOAD_MODE: 'sample' + }); + $timeout.flush(); + expect(ctrl.imageSourceOptions).toEqual([fileSourceOption, urlSourceOption]); + }); + }); + }); + + describe('isLocalFileUpload()', function() { + var ctrl; + + beforeEach(function() { + ctrl = createController(); + }); + + it('returns true for source-type == "file-direct"', function() { + ctrl.image = {source_type: 'file-direct'}; + expect(ctrl.isLocalFileUpload()).toBe(true); + }); + + it('returns true for source-type == "file-legacy"', function() { + ctrl.image = {source_type: 'file-legacy'}; + expect(ctrl.isLocalFileUpload()).toBe(true); + }); + + it('returns false for any else source-type', function() { + ctrl.image = {source_type: 'url'}; + expect(ctrl.isLocalFileUpload()).toBe(false); + }); + }); }); diff --git a/openstack_dashboard/static/app/core/images/steps/create-image/create-image.html b/openstack_dashboard/static/app/core/images/steps/create-image/create-image.html index e47d719c31..f6c1536d17 100644 --- a/openstack_dashboard/static/app/core/images/steps/create-image/create-image.html +++ b/openstack_dashboard/static/app/core/images/steps/create-image/create-image.html @@ -52,7 +52,46 @@
-
+
+
+ +
+
+ +
+
+
+
+
+
+
+
+ +
+ + + + +
+

+ A local file should be selected. +

+
+
+
+
+