From 3460e83c26fed55f68141cb1caecf6c172883295 Mon Sep 17 00:00:00 2001 From: Nizamudeen A Date: Fri, 22 Sep 2023 18:07:39 +0530 Subject: [PATCH] mgr/dashboard: fix cephfs forms validations 1. CephFS Edit Form didnt had any validation for name eventhough the create had. So reused the Create form to display the Edit as well 2. Add Name Validations to Subvoume and Subvolume group forms 3. Removed the datePipe from the cephfs list template since we are using the relativeDate. Fixes: https://tracker.ceph.com/issues/62939 Signed-off-by: Nizamudeen A (cherry picked from commit cd617a3bd28b0aedd03505314509f0c00391bc39) --- .../filesystems/filesystems.e2e-spec.feature | 2 +- .../frontend/src/app/app-routing.module.ts | 5 + .../cephfs-form/cephfs-form.component.html | 6 +- .../cephfs-form/cephfs-form.component.ts | 127 +++++++++++------- .../cephfs-list/cephfs-list.component.ts | 34 +---- .../cephfs-subvolume-form.component.html | 3 + .../cephfs-subvolume-form.component.ts | 2 +- .../cephfs-subvolumegroup-form.component.html | 3 + .../cephfs-subvolumegroup-form.component.ts | 2 +- .../shared/services/task-message.service.ts | 3 + 10 files changed, 101 insertions(+), 86 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/cypress/e2e/filesystems/filesystems.e2e-spec.feature b/src/pybind/mgr/dashboard/frontend/cypress/e2e/filesystems/filesystems.e2e-spec.feature index 79685299b93..2c08fb56eff 100644 --- a/src/pybind/mgr/dashboard/frontend/cypress/e2e/filesystems/filesystems.e2e-spec.feature +++ b/src/pybind/mgr/dashboard/frontend/cypress/e2e/filesystems/filesystems.e2e-spec.feature @@ -16,7 +16,7 @@ Feature: CephFS Management Given I am on the "cephfs" page And I select a row "test_cephfs" And I click on "Edit" button - And enter "name" "test_cephfs_edit" in the modal + And enter "name" "test_cephfs_edit" And I click on "Edit File System" button Then I should see a row with "test_cephfs_edit" diff --git a/src/pybind/mgr/dashboard/frontend/src/app/app-routing.module.ts b/src/pybind/mgr/dashboard/frontend/src/app/app-routing.module.ts index 2e5553414ba..63a58d8ee20 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/app-routing.module.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/app-routing.module.ts @@ -359,6 +359,11 @@ const routes: Routes = [ path: URLVerbs.CREATE, component: CephfsVolumeFormComponent, data: { breadcrumbs: ActionLabels.CREATE } + }, + { + path: `${URLVerbs.EDIT}/:name`, + component: CephfsVolumeFormComponent, + data: { breadcrumbs: ActionLabels.EDIT } } ] }, diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-form/cephfs-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-form/cephfs-form.component.html index f1bcd915571..76e51b2c5f3 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-form/cephfs-form.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-form/cephfs-form.component.html @@ -12,7 +12,8 @@ Orchestrator is not configured. Deploy MDS daemons manually after creating the volume. + i18n + *ngIf="!editing">Orchestrator is not configured. Deploy MDS daemons manually after creating the volume.
@@ -40,7 +41,8 @@ -
+
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-form/cephfs-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-form/cephfs-form.component.ts index 7c607fa33d8..6d84e33c7b6 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-form/cephfs-form.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-form/cephfs-form.component.ts @@ -1,6 +1,6 @@ import { Component, OnInit, ViewChild } from '@angular/core'; import { FormControl, Validators } from '@angular/forms'; -import { Router } from '@angular/router'; +import { ActivatedRoute, Router } from '@angular/router'; import _ from 'lodash'; import { NgbNav, NgbTooltip, NgbTypeahead } from '@ng-bootstrap/ng-bootstrap'; @@ -50,6 +50,7 @@ export class CephfsVolumeFormComponent extends CdForm implements OnInit { hosts: any; labels: string[]; hasOrchestrator: boolean; + currentVolumeName: string; constructor( private router: Router, @@ -58,10 +59,11 @@ export class CephfsVolumeFormComponent extends CdForm implements OnInit { private formBuilder: CdFormBuilder, public actionLabels: ActionLabelsI18n, private hostService: HostService, - private cephfsService: CephfsService + private cephfsService: CephfsService, + private route: ActivatedRoute ) { super(); - this.editing = this.router.url.startsWith(`/pool/${URLVerbs.EDIT}`); + this.editing = this.router.url.startsWith(`/cephfs/${URLVerbs.EDIT}`); this.action = this.editing ? this.actionLabels.EDIT : this.actionLabels.CREATE; this.resource = $localize`File System`; this.hosts = { @@ -98,20 +100,27 @@ export class CephfsVolumeFormComponent extends CdForm implements OnInit { } ngOnInit() { - const hostContext = new CdTableFetchDataContext(() => undefined); - this.hostService.list(hostContext.toParams(), 'false').subscribe((resp: object[]) => { - const options: SelectOption[] = []; - _.forEach(resp, (host: object) => { - if (_.get(host, 'sources.orchestrator', false)) { - const option = new SelectOption(false, _.get(host, 'hostname'), ''); - options.push(option); - } + if (this.editing) { + this.route.params.subscribe((params: { name: string }) => { + this.currentVolumeName = params.name; + this.form.get('name').setValue(this.currentVolumeName); }); - this.hosts.options = [...options]; - }); - this.hostService.getLabels().subscribe((resp: string[]) => { - this.labels = resp; - }); + } else { + const hostContext = new CdTableFetchDataContext(() => undefined); + this.hostService.list(hostContext.toParams(), 'false').subscribe((resp: object[]) => { + const options: SelectOption[] = []; + _.forEach(resp, (host: object) => { + if (_.get(host, 'sources.orchestrator', false)) { + const option = new SelectOption(false, _.get(host, 'hostname'), ''); + options.push(option); + } + }); + this.hosts.options = [...options]; + }); + this.hostService.getLabels().subscribe((resp: string[]) => { + this.labels = resp; + }); + } this.orchStatus$ = this.orchService.status(); } @@ -130,39 +139,59 @@ export class CephfsVolumeFormComponent extends CdForm implements OnInit { }; submit() { - let values = this.form.getRawValue(); - const serviceSpec: object = { - placement: {}, - unmanaged: values['unmanaged'] - }; - switch (values['placement']) { - case 'hosts': - if (values['hosts'].length > 0) { - serviceSpec['placement']['hosts'] = values['hosts']; - } - break; - case 'label': - serviceSpec['placement']['label'] = values['label']; - break; - } - const volumeName = this.form.get('name').value; - const self = this; - let taskUrl = `cephfs/${URLVerbs.CREATE}`; - this.taskWrapperService - .wrapTaskAroundCall({ - task: new FinishedTask(taskUrl, { - volumeName: volumeName - }), - call: this.cephfsService.create(this.form.get('name').value, serviceSpec) - }) - .subscribe({ - error() { - self.form.setErrors({ cdSubmitButton: true }); - }, - complete: () => { - this.router.navigate(['cephfs']); - } - }); + const BASE_URL = 'cephfs'; + + if (this.editing) { + this.taskWrapperService + .wrapTaskAroundCall({ + task: new FinishedTask(`${BASE_URL}/${URLVerbs.EDIT}`, { + volumeName: volumeName + }), + call: this.cephfsService.rename(this.currentVolumeName, volumeName) + }) + .subscribe({ + error: () => { + this.form.setErrors({ cdSubmitButton: true }); + }, + complete: () => { + this.router.navigate([BASE_URL]); + } + }); + } else { + let values = this.form.getRawValue(); + const serviceSpec: object = { + placement: {}, + unmanaged: values['unmanaged'] + }; + switch (values['placement']) { + case 'hosts': + if (values['hosts'].length > 0) { + serviceSpec['placement']['hosts'] = values['hosts']; + } + break; + case 'label': + serviceSpec['placement']['label'] = values['label']; + break; + } + + const self = this; + let taskUrl = `${BASE_URL}/${URLVerbs.CREATE}`; + this.taskWrapperService + .wrapTaskAroundCall({ + task: new FinishedTask(taskUrl, { + volumeName: volumeName + }), + call: this.cephfsService.create(this.form.get('name').value, serviceSpec) + }) + .subscribe({ + error() { + self.form.setErrors({ cdSubmitButton: true }); + }, + complete: () => { + this.router.navigate([BASE_URL]); + } + }); + } } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-list/cephfs-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-list/cephfs-list.component.ts index 280189ece62..0d55845ab59 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-list/cephfs-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-list/cephfs-list.component.ts @@ -11,13 +11,10 @@ import { CellTemplate } from '~/app/shared/enum/cell-template.enum'; import { ActionLabelsI18n } from '~/app/shared/constants/app.constants'; import { Icons } from '~/app/shared/enum/icons.enum'; import { CriticalConfirmationModalComponent } from '~/app/shared/components/critical-confirmation-modal/critical-confirmation-modal.component'; -import { NotificationType } from '~/app/shared/enum/notification-type.enum'; -import { FormModalComponent } from '~/app/shared/components/form-modal/form-modal.component'; import { CdTableAction } from '~/app/shared/models/cd-table-action'; import { CdTableColumn } from '~/app/shared/models/cd-table-column'; import { CdTableFetchDataContext } from '~/app/shared/models/cd-table-fetch-data-context'; import { CdTableSelection } from '~/app/shared/models/cd-table-selection'; -import { CdDatePipe } from '~/app/shared/pipes/cd-date.pipe'; import { AuthStorageService } from '~/app/shared/services/auth-storage.service'; import { URLBuilderService } from '~/app/shared/services/url-builder.service'; import { ModalService } from '~/app/shared/services/modal.service'; @@ -45,7 +42,6 @@ export class CephfsListComponent extends ListWithDetails implements OnInit { constructor( private authStorageService: AuthStorageService, private cephfsService: CephfsService, - private cdDatePipe: CdDatePipe, public actionLabels: ActionLabelsI18n, private router: Router, private urlBuilder: URLBuilderService, @@ -75,7 +71,6 @@ export class CephfsListComponent extends ListWithDetails implements OnInit { name: $localize`Created`, prop: 'mdsmap.created', flexGrow: 1, - pipe: this.cdDatePipe, cellTransformation: CellTemplate.timeAgo } ]; @@ -91,7 +86,8 @@ export class CephfsListComponent extends ListWithDetails implements OnInit { name: this.actionLabels.EDIT, permission: 'update', icon: Icons.edit, - click: () => this.editAction() + click: () => + this.router.navigate([this.urlBuilder.getEdit(this.selection.first().mdsmap.fs_name)]) }, { permission: 'delete', @@ -154,30 +150,4 @@ export class CephfsListComponent extends ListWithDetails implements OnInit { return true; } - - editAction() { - const selectedVolume = this.selection.first().mdsmap['fs_name']; - - this.modalService.show(FormModalComponent, { - titleText: $localize`Edit File System: ${selectedVolume}`, - fields: [ - { - type: 'text', - name: 'name', - value: selectedVolume, - label: $localize`Name`, - required: true - } - ], - submitButtonText: $localize`Edit File System`, - onSubmit: (values: any) => { - this.cephfsService.rename(selectedVolume, values.name).subscribe(() => { - this.notificationService.show( - NotificationType.success, - $localize`Updated File System '${selectedVolume}'` - ); - }); - } - }); - } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolume-form/cephfs-subvolume-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolume-form/cephfs-subvolume-form.component.html index ad6f414486d..a810b7e5d2f 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolume-form/cephfs-subvolume-form.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolume-form/cephfs-subvolume-form.component.html @@ -27,6 +27,9 @@ The subvolume already exists. + Subvolume name can only contain letters, numbers, '.', '-' or '_'
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolume-form/cephfs-subvolume-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolume-form/cephfs-subvolume-form.component.ts index b85c268326a..2c2fe8f9fa0 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolume-form/cephfs-subvolume-form.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolume-form/cephfs-subvolume-form.component.ts @@ -101,7 +101,7 @@ export class CephfsSubvolumeFormComponent extends CdForm implements OnInit { this.subvolumeForm = new CdFormGroup({ volumeName: new FormControl({ value: this.fsName, disabled: true }), subvolumeName: new FormControl('', { - validators: [Validators.required], + validators: [Validators.required, Validators.pattern(/^[.A-Za-z0-9_-]+$/)], asyncValidators: [ CdValidators.unique( this.cephFsSubvolumeService.exists, diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolumegroup-form/cephfs-subvolumegroup-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolumegroup-form/cephfs-subvolumegroup-form.component.html index 1b451dfbb37..58bb86021bd 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolumegroup-form/cephfs-subvolumegroup-form.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolumegroup-form/cephfs-subvolumegroup-form.component.html @@ -27,6 +27,9 @@ The subvolume group already exists. + Subvolume name can only contain letters, numbers, '.', '-' or '_'
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolumegroup-form/cephfs-subvolumegroup-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolumegroup-form/cephfs-subvolumegroup-form.component.ts index f7d5c4802a4..8ecf1eafa8c 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolumegroup-form/cephfs-subvolumegroup-form.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cephfs/cephfs-subvolumegroup-form/cephfs-subvolumegroup-form.component.ts @@ -93,7 +93,7 @@ export class CephfsSubvolumegroupFormComponent extends CdForm implements OnInit this.subvolumegroupForm = new CdFormGroup({ volumeName: new FormControl({ value: this.fsName, disabled: true }), subvolumegroupName: new FormControl('', { - validators: [Validators.required], + validators: [Validators.required, Validators.pattern(/^[.A-Za-z0-9_-]+$/)], asyncValidators: [ CdValidators.unique( this.cephfsSubvolumeGroupService.exists, diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-message.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-message.service.ts index 023a1529fa7..f6969c2e8e1 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-message.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-message.service.ts @@ -356,6 +356,9 @@ export class TaskMessageService { 'cephfs/create': this.newTaskMessage(this.commonOperations.create, (metadata) => this.volume(metadata) ), + 'cephfs/edit': this.newTaskMessage(this.commonOperations.update, (metadata) => + this.volume(metadata) + ), 'cephfs/remove': this.newTaskMessage(this.commonOperations.remove, (metadata) => this.volume(metadata) ), -- 2.39.5