From 961ba7ea1c4c10b228b54c03f855fd3440d8420e Mon Sep 17 00:00:00 2001 From: Ricardo Marques Date: Tue, 9 Jul 2019 15:12:24 +0100 Subject: [PATCH] mgr/dashboard: Validate iSCSI controls min/max value Fixes: https://tracker.ceph.com/issues/38018 Signed-off-by: Ricardo Marques --- src/pybind/mgr/dashboard/controllers/iscsi.py | 46 ++++++++- .../iscsi-target-form.component.ts | 8 +- ...target-image-settings-modal.component.html | 97 +++++++++++-------- ...get-image-settings-modal.component.spec.ts | 15 +-- ...i-target-image-settings-modal.component.ts | 47 +++++++-- ...i-target-iqn-settings-modal.component.html | 8 ++ ...csi-target-iqn-settings-modal.component.ts | 14 ++- 7 files changed, 173 insertions(+), 62 deletions(-) diff --git a/src/pybind/mgr/dashboard/controllers/iscsi.py b/src/pybind/mgr/dashboard/controllers/iscsi.py index df44f88980704..92a32c3db80c2 100644 --- a/src/pybind/mgr/dashboard/controllers/iscsi.py +++ b/src/pybind/mgr/dashboard/controllers/iscsi.py @@ -223,7 +223,7 @@ class IscsiTarget(RESTController): raise DashboardException(msg='Target already exists', code='target_already_exists', component='iscsi') - IscsiTarget._validate(target_iqn, portals, disks, groups) + IscsiTarget._validate(target_iqn, target_controls, portals, disks, groups) IscsiTarget._create(target_iqn, target_controls, acl_enabled, portals, disks, clients, groups, 0, 100, config) @@ -245,7 +245,7 @@ class IscsiTarget(RESTController): raise DashboardException(msg='Target IQN already in use', code='target_iqn_already_in_use', component='iscsi') - IscsiTarget._validate(new_target_iqn, portals, disks, groups) + IscsiTarget._validate(new_target_iqn, target_controls, portals, disks, groups) config = IscsiTarget._delete(target_iqn, config, 0, 50, new_target_iqn, target_controls, portals, disks, clients, groups) IscsiTarget._create(new_target_iqn, target_controls, acl_enabled, portals, disks, clients, @@ -412,7 +412,7 @@ class IscsiTarget(RESTController): return False @staticmethod - def _validate(target_iqn, portals, disks, groups): + def _validate(target_iqn, target_controls, portals, disks, groups): if not target_iqn: raise DashboardException(msg='Target IQN is required', code='target_iqn_required', @@ -430,6 +430,26 @@ class IscsiTarget(RESTController): code='portals_required', component='iscsi') + # 'target_controls_limits' was introduced in ceph-iscsi > 3.2 + # When using an older `ceph-iscsi` version these validations will + # NOT be executed beforehand + if 'target_controls_limits' in settings: + for target_control_name, target_control_value in target_controls.items(): + limits = settings['target_controls_limits'].get(target_control_name) + if limits is not None: + min_value = limits.get('min') + if min_value is not None and target_control_value < min_value: + raise DashboardException(msg='Target control {} must be >= ' + '{}'.format(target_control_name, min_value), + code='target_control_invalid_min', + component='iscsi') + max_value = limits.get('max') + if max_value is not None and target_control_value > max_value: + raise DashboardException(msg='Target control {} must be <= ' + '{}'.format(target_control_name, max_value), + code='target_control_invalid_max', + component='iscsi') + for portal in portals: gateway_name = portal['host'] try: @@ -449,6 +469,26 @@ class IscsiTarget(RESTController): IscsiTarget._validate_image(pool, image, backstore, required_rbd_features, unsupported_rbd_features) + # 'disk_controls_limits' was introduced in ceph-iscsi > 3.2 + # When using an older `ceph-iscsi` version these validations will + # NOT be executed beforehand + if 'disk_controls_limits' in settings: + for disk_control_name, disk_control_value in disk['controls'].items(): + limits = settings['disk_controls_limits'][backstore].get(disk_control_name) + if limits is not None: + min_value = limits.get('min') + if min_value is not None and disk_control_value < min_value: + raise DashboardException(msg='Disk control {} must be >= ' + '{}'.format(disk_control_name, min_value), + code='disk_control_invalid_min', + component='iscsi') + max_value = limits.get('max') + if max_value is not None and disk_control_value > max_value: + raise DashboardException(msg='Disk control {} must be <= ' + '{}'.format(disk_control_name, max_value), + code='disk_control_invalid_max', + component='iscsi') + initiators = [] for group in groups: initiators = initiators + group['members'] diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-form/iscsi-target-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-form/iscsi-target-form.component.ts index 1092c7c49d52b..576752539dc08 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-form/iscsi-target-form.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-form/iscsi-target-form.component.ts @@ -30,7 +30,9 @@ export class IscsiTargetFormComponent implements OnInit { modalRef: BsModalRef; minimum_gateways = 1; target_default_controls: any; + target_controls_limits: any; disk_default_controls: any; + disk_controls_limits: any; backstores: string[]; default_backstore: string; unsupported_rbd_features: any; @@ -124,7 +126,9 @@ export class IscsiTargetFormComponent implements OnInit { // iscsiService.settings() this.minimum_gateways = data[3].config.minimum_gateways; this.target_default_controls = data[3].target_default_controls; + this.target_controls_limits = data[3].target_controls_limits; this.disk_default_controls = data[3].disk_default_controls; + this.disk_controls_limits = data[3].disk_controls_limits; this.backstores = data[3].backstores; this.default_backstore = data[3].default_backstore; this.unsupported_rbd_features = data[3].unsupported_rbd_features; @@ -666,7 +670,8 @@ export class IscsiTargetFormComponent implements OnInit { targetSettingsModal() { const initialState = { target_controls: this.targetForm.get('target_controls'), - target_default_controls: this.target_default_controls + target_default_controls: this.target_default_controls, + target_controls_limits: this.target_controls_limits }; this.modalRef = this.modalService.show(IscsiTargetIqnSettingsModalComponent, { initialState }); @@ -677,6 +682,7 @@ export class IscsiTargetFormComponent implements OnInit { imagesSettings: this.imagesSettings, image: image, disk_default_controls: this.disk_default_controls, + disk_controls_limits: this.disk_controls_limits, backstores: this.getValidBackstores(this.getImageById(image)) }; diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.html index 438046184e58d..ecfc41c317b3d 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.html @@ -5,54 +5,67 @@ -
-

Changing these parameters from their default values is usually not necessary.

+
+
+

Changing these parameters from their default values is usually not necessary.

- -
-
- - + +
+
+ + +
-
- - - -
-
- - - {{ helpText[setting.key]?.help }} + + + +
+
+ + + + Must be greater than or equal to {{ disk_controls_limits[bs][setting.key]['min'] }}. + + + Must be less than or equal to {{ disk_controls_limits[bs][setting.key]['max'] }}. + + {{ helpText[setting.key]?.help }} +
-
+ - -
+
-
-
- - - +
+
+ Confirm + + +
-
+ diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.spec.ts index 00f4da5fa07dc..c25b73c00575d 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.spec.ts @@ -1,6 +1,6 @@ import { HttpClientTestingModule } from '@angular/common/http/testing'; import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { FormsModule } from '@angular/forms'; +import { ReactiveFormsModule } from '@angular/forms'; import { RouterTestingModule } from '@angular/router/testing'; import { BsModalRef } from 'ngx-bootstrap/modal'; @@ -15,7 +15,7 @@ describe('IscsiTargetImageSettingsModalComponent', () => { configureTestBed({ declarations: [IscsiTargetImageSettingsModalComponent], - imports: [SharedModule, FormsModule, HttpClientTestingModule, RouterTestingModule], + imports: [SharedModule, ReactiveFormsModule, HttpClientTestingModule, RouterTestingModule], providers: [BsModalRef, i18nProviders] }); @@ -44,16 +44,17 @@ describe('IscsiTargetImageSettingsModalComponent', () => { expect(component).toBeTruthy(); }); - it('should fill the model', () => { - expect(component.model).toEqual({ + it('should fill the form', () => { + expect(component.settingsForm.value).toEqual({ backstore: 'backstore:1', - 'backstore:1': {}, - 'backstore:2': {} + foo: null, + bar: null, + baz: null }); }); it('should save changes to imagesSettings', () => { - component.model['backstore:1'] = { foo: 1234 }; + component.settingsForm.controls['foo'].setValue(1234); expect(component.imagesSettings).toEqual({ 'rbd/disk_1': { backstore: 'backstore:1', 'backstore:1': {} } }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.ts index 310942a590ab7..e2687300dc9b5 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.ts @@ -1,9 +1,11 @@ import { Component, OnInit } from '@angular/core'; +import { FormControl, Validators } from '@angular/forms'; import * as _ from 'lodash'; import { BsModalRef } from 'ngx-bootstrap/modal'; import { IscsiService } from '../../../shared/api/iscsi.service'; +import { CdFormGroup } from '../../../shared/forms/cd-form-group'; @Component({ selector: 'cd-iscsi-target-image-settings-modal', @@ -14,9 +16,10 @@ export class IscsiTargetImageSettingsModalComponent implements OnInit { image: string; imagesSettings: any; disk_default_controls: any; + disk_controls_limits: any; backstores: any; - model: any; + settingsForm: CdFormGroup; helpText: any; constructor(public modalRef: BsModalRef, public iscsiService: IscsiService) {} @@ -24,18 +27,48 @@ export class IscsiTargetImageSettingsModalComponent implements OnInit { ngOnInit() { this.helpText = this.iscsiService.imageAdvancedSettings; - this.model = _.cloneDeep(this.imagesSettings[this.image]); + const fg = { + backstore: new FormControl(this.imagesSettings[this.image]['backstore']) + }; _.forEach(this.backstores, (backstore) => { - this.model[backstore] = this.model[backstore] || {}; + const model = this.imagesSettings[this.image][backstore] || {}; + _.forIn(this.disk_default_controls[backstore], (_value, key) => { + const validators = []; + if (this.disk_controls_limits && key in this.disk_controls_limits[backstore]) { + if ('min' in this.disk_controls_limits[backstore][key]) { + validators.push(Validators.min(this.disk_controls_limits[backstore][key]['min'])); + } + if ('max' in this.disk_controls_limits[backstore][key]) { + validators.push(Validators.max(this.disk_controls_limits[backstore][key]['max'])); + } + } + fg[key] = new FormControl(model[key], { + validators: validators + }); + }); }); + + this.settingsForm = new CdFormGroup(fg); } save() { - const backstore = this.model.backstore; + const backstore = this.settingsForm.controls['backstore'].value; const settings = {}; - _.forIn(this.model[backstore], (value, key) => { - if (!(value === '' || value === null)) { - settings[key] = value; + _.forIn(this.settingsForm.controls, (control, key) => { + if ( + !(control.value === '' || control.value === null) && + key in this.disk_default_controls[this.settingsForm.value['backstore']] + ) { + settings[key] = control.value; + // If one setting belongs to multiple backstores, we have to update it in all backstores + _.forEach(this.backstores, (currentBackstore) => { + if (currentBackstore !== backstore) { + const model = this.imagesSettings[this.image][currentBackstore] || {}; + if (key in model) { + this.imagesSettings[this.image][currentBackstore][key] = control.value; + } + } + }); } }); this.imagesSettings[this.image]['backstore'] = backstore; diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-iqn-settings-modal/iscsi-target-iqn-settings-modal.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-iqn-settings-modal/iscsi-target-iqn-settings-modal.component.html index abfe37c9d9d97..8d00c80ce4761 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-iqn-settings-modal/iscsi-target-iqn-settings-modal.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-iqn-settings-modal/iscsi-target-iqn-settings-modal.component.html @@ -20,6 +20,14 @@ *ngIf="!isRadio(setting.key)" type="number" [formControlName]="setting.key"> + + Must be greater than or equal to {{ target_controls_limits[setting.key]['min'] }}. + + + Must be less than or equal to {{ target_controls_limits[setting.key]['max'] }}. +
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-iqn-settings-modal/iscsi-target-iqn-settings-modal.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-iqn-settings-modal/iscsi-target-iqn-settings-modal.component.ts index eef5276cb73da..622beb4619e4c 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-iqn-settings-modal/iscsi-target-iqn-settings-modal.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-iqn-settings-modal/iscsi-target-iqn-settings-modal.component.ts @@ -1,5 +1,5 @@ import { Component, OnInit } from '@angular/core'; -import { FormControl } from '@angular/forms'; +import { FormControl, Validators } from '@angular/forms'; import * as _ from 'lodash'; import { BsModalRef } from 'ngx-bootstrap/modal'; @@ -15,6 +15,7 @@ import { CdFormGroup } from '../../../shared/forms/cd-form-group'; export class IscsiTargetIqnSettingsModalComponent implements OnInit { target_controls: FormControl; target_default_controls: any; + target_controls_limits: any; settingsForm: CdFormGroup; helpText: any; @@ -26,7 +27,16 @@ export class IscsiTargetIqnSettingsModalComponent implements OnInit { this.helpText = this.iscsiService.targetAdvancedSettings; _.forIn(this.target_default_controls, (_value, key) => { - fg[key] = new FormControl(this.target_controls.value[key]); + const validators = []; + if (this.target_controls_limits && key in this.target_controls_limits) { + if ('min' in this.target_controls_limits[key]) { + validators.push(Validators.min(this.target_controls_limits[key]['min'])); + } + if ('max' in this.target_controls_limits[key]) { + validators.push(Validators.max(this.target_controls_limits[key]['max'])); + } + } + fg[key] = new FormControl(this.target_controls.value[key], { validators: validators }); }); this.settingsForm = new CdFormGroup(fg); -- 2.39.5