Skip to content

Commit

Permalink
Lazy load StorageManagers upon volume creation
Browse files Browse the repository at this point in the history
When creating a new volume user has to pick what StorageManager is volume
supposed to be created with, hence we perform API request to fetch those.
Problem is we share angular controller with other volume operations (attach,
detach) that don't need this list of all managers and fetching those
anyway results in permission denied error due to fine-grained permission
setting.

For example if we give user only permission to attach/detach volume, but
not to view StorageManagers, then we must not perform the API call as it
results in 403 with red popup.

With this commit we refactor controller so that it now only performs the
API calls that are really needed.

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1571224

Signed-off-by: Miha Pleško <miha.plesko@xlab.si>
  • Loading branch information
miha-plesko committed Aug 31, 2018
1 parent 8e56e43 commit 771e97f
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 29 deletions.
@@ -1,4 +1,4 @@
ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API', 'cloudVolumeFormId', 'storageManagerId', '$timeout', function(miqService, API, cloudVolumeFormId, storageManagerId, $timeout) {
ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API', 'cloudVolumeFormId', 'cloudVolumeFormOperation', 'storageManagerId', '$timeout', '$q', function(miqService, API, cloudVolumeFormId, cloudVolumeFormOperation, storageManagerId, $timeout, $q) {
var vm = this;

var init = function() {
Expand All @@ -21,19 +21,32 @@ ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API
vm.newRecord = cloudVolumeFormId === 'new';

miqService.sparkleOn();
API.get('/api/providers?expand=resources&attributes=id,name,supports_block_storage&filter[]=supports_block_storage=true')
.then(getStorageManagers)
.catch(miqService.handleFailure);

if (cloudVolumeFormId !== 'new') {
// Fetch cloud volume data before showing the form.
API.get('/api/cloud_volumes/' + cloudVolumeFormId + '?attributes=ext_management_system.type,availability_zone.ems_ref,base_snapshot.ems_ref')
.then(getCloudVolumeFormData)
.catch(miqService.handleFailure);
} else {
// Initialise the form immediately when creating a new volume.
setForm();
// Load initial API data depending on what form we're displaying. Do as little requests as possible
// to support fine-grained API permissions.
var apiPromises = [];
switch (cloudVolumeFormOperation) {
case 'EDIT':
// Fetch relevant StorageManager, just enough to populate the disabled drop-down.
apiPromises.push(API.get('/api/providers/' + storageManagerId + '?attributes=id,name')
.then(getStorageManagers));
// Limit form options as permitted by this StorageManager.
apiPromises.push(vm.storageManagerChanged(storageManagerId));
// Fetch the volume.
apiPromises.push(loadVolume(cloudVolumeFormId));
break;
case 'NEW':
// Fetch StorageManagers that we can even create the new volume for.
apiPromises.push(API.get('/api/providers?expand=resources&attributes=id,name,supports_block_storage&filter[]=supports_block_storage=true')
.then(getStorageManagers));
break;
default:
// Fetch the volume.
apiPromises.push(loadVolume(cloudVolumeFormId));
}

// After all the API data is at hand we show the form.
$q.all(apiPromises).then(setForm).catch(miqService.handleFailure);
};

vm.addClicked = function() {
Expand Down Expand Up @@ -115,7 +128,7 @@ ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API

vm.storageManagerChanged = function(id) {
miqService.sparkleOn();
API.get('/api/providers/' + id + '?attributes=type,parent_manager.availability_zones,parent_manager.cloud_tenants,parent_manager.cloud_volume_snapshots')
return API.get('/api/providers/' + id + '?attributes=type,parent_manager.availability_zones,parent_manager.cloud_tenants,parent_manager.cloud_volume_snapshots')
.then(getStorageManagerFormData)
.catch(miqService.handleFailure);
};
Expand Down Expand Up @@ -193,6 +206,11 @@ ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API
miqService.sparkleOff();
}

var loadVolume = function(id) {
return API.get('/api/cloud_volumes/' + id + '?attributes=ext_management_system.type,availability_zone.ems_ref,base_snapshot.ems_ref')
.then(getCloudVolumeFormData)
};

var loadEBSVolumeTypes = function() {
// This ia a fixed list of available cloud volume types for Amazon EBS.
vm.awsVolumeTypes = [
Expand All @@ -212,13 +230,8 @@ ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API
};

var getStorageManagers = function(data) {
vm.storageManagers = data.resources;

// If storage manager ID was provided, we need to refresh the form and show
// corresponding form fields.
if (storageManagerId) {
vm.storageManagerChanged(vm.cloudVolumeModel.storage_manager_id);
}
// Can handle list of all managers or a single manager.
vm.storageManagers = data.resources ? data.resources : [data];
};

var getCloudVolumeFormData = function(data) {
Expand All @@ -243,8 +256,6 @@ ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API
if (angular.isUndefined(vm.cloudVolumeModel.aws_iops)) {
vm.sizeChanged(vm.cloudVolumeModel.size);
}

setForm();
};

var getStorageManagerFormData = function(data) {
Expand Down
3 changes: 2 additions & 1 deletion app/views/cloud_volume/attach.html.haml
Expand Up @@ -56,5 +56,6 @@
:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id}');
ManageIQ.angular.app.value('storageManagerId', #{@volume.ext_management_system.id});
ManageIQ.angular.app.value('storageManagerId', '#{@volume.ext_management_system.id.to_s}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'ATTACH');
miq_bootstrap('#form_div');
3 changes: 2 additions & 1 deletion app/views/cloud_volume/backup_new.html.haml
Expand Up @@ -62,5 +62,6 @@
:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id}');
ManageIQ.angular.app.value('storageManagerId', #{@volume.ext_management_system.id});
ManageIQ.angular.app.value('storageManagerId', '#{@volume.ext_management_system.id.to_s}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'BACKUP_NEW');
miq_bootstrap(jQuery('#form_div'));
3 changes: 2 additions & 1 deletion app/views/cloud_volume/backup_select.html.haml
Expand Up @@ -39,5 +39,6 @@
:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id}');
ManageIQ.angular.app.value('storageManagerId', #{@volume.ext_management_system.id});
ManageIQ.angular.app.value('storageManagerId', '#{@volume.ext_management_system.id.to_s}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'BACKUP_SELECT');
miq_bootstrap(jQuery('#form_div'));
3 changes: 2 additions & 1 deletion app/views/cloud_volume/detach.html.haml
Expand Up @@ -36,5 +36,6 @@
:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id}');
ManageIQ.angular.app.value('storageManagerId', #{@volume.ext_management_system.id});
ManageIQ.angular.app.value('storageManagerId', '#{@volume.ext_management_system.id.to_s}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'DETACH');
miq_bootstrap('#form_div');
3 changes: 2 additions & 1 deletion app/views/cloud_volume/edit.html.haml
Expand Up @@ -13,5 +13,6 @@
:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id}');
ManageIQ.angular.app.value('storageManagerId', #{@volume.ext_management_system.id});
ManageIQ.angular.app.value('storageManagerId', '#{@volume.ext_management_system.id.to_s}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'EDIT');
miq_bootstrap(jQuery('#form_div'));
3 changes: 2 additions & 1 deletion app/views/cloud_volume/new.html.haml
Expand Up @@ -11,5 +11,6 @@
:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id || "new"}');
ManageIQ.angular.app.value('storageManagerId', #{@storage_manager.try(:id) || "undefined"});
ManageIQ.angular.app.value('storageManagerId', '#{@storage_manager.try(:id).to_s || "undefined"}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'NEW');
miq_bootstrap('#form_div');
3 changes: 2 additions & 1 deletion app/views/cloud_volume/snapshot_new.html.haml
Expand Up @@ -39,5 +39,6 @@
:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id}');
ManageIQ.angular.app.value('storageManagerId', #{@volume.ext_management_system.id});
ManageIQ.angular.app.value('storageManagerId', '#{@volume.ext_management_system.id.to_s}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'SNAPSHOT_NEW');
miq_bootstrap(jQuery('#form_div'));

0 comments on commit 771e97f

Please sign in to comment.