From aaa0c77652ca2c8cce513c5a802f16f1904256c6 Mon Sep 17 00:00:00 2001 From: Ido Rosenzwig Date: Wed, 22 Jan 2020 19:10:46 +0200 Subject: [PATCH] Show only the allowed storage interfaces on Disk-modal Fixes: CNV-2914 Enforce the validations from common and user templates so the user can choose only from the allowed values in the Disk-modal at the storage tab. Depends on: #3909 Signed-off-by: Ido Rosenzwig --- .../validations/storage-tab-validation.ts | 5 +++ .../validations/vm-settings-tab-validation.ts | 6 ++-- .../template.ts} | 32 +++++++++++-------- .../vm-wizard-storage-modal-enhanced.tsx | 11 +++++++ .../storage-tab/vm-wizard-storage-row.tsx | 1 + .../modals/disk-modal/disk-modal.tsx | 29 +++++++++++++++-- .../src/utils/validations/common.ts | 14 ++++++++ .../template/interval-validation-result.ts | 2 +- .../template/template-validations.ts | 25 ++++++++++++++- .../src/utils/validations/vm/disk.ts | 15 +++++++-- 10 files changed, 117 insertions(+), 23 deletions(-) rename frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/{redux/validations/utils/templates-validations.ts => selectors/template.ts} (59%) diff --git a/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/redux/validations/storage-tab-validation.ts b/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/redux/validations/storage-tab-validation.ts index 1b2d9f71517..2459064a269 100644 --- a/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/redux/validations/storage-tab-validation.ts +++ b/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/redux/validations/storage-tab-validation.ts @@ -8,6 +8,8 @@ import { getStoragesWithWrappers } from '../../selectors/selectors'; import { iGetStorages } from '../../selectors/immutable/storage'; import { iGetProvisionSource } from '../../selectors/immutable/vm-settings'; import { ProvisionSource } from '../../../../constants/vm/provision-source'; +import { TemplateValidations } from '../../../../utils/validations/template/template-validations'; +import { getTemplateValidation } from '../../selectors/template'; export const validateStorages = (options: UpdateOptions) => { const { id, prevState, dispatch, getState } = options; @@ -28,6 +30,8 @@ export const validateStorages = (options: UpdateOptions) => { } const storages = getStoragesWithWrappers(state, id); + const templateValidation = getTemplateValidation(state, id); + const allowedBusses = (templateValidation || new TemplateValidations()).getAllowedBusses(); const validatedStorages = storages.map( ({ @@ -56,6 +60,7 @@ export const validateStorages = (options: UpdateOptions) => { { usedDiskNames, usedPVCNames, + allowedBusses, }, ), }; diff --git a/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/redux/validations/vm-settings-tab-validation.ts b/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/redux/validations/vm-settings-tab-validation.ts index af5c6fea923..4aff038c9bc 100644 --- a/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/redux/validations/vm-settings-tab-validation.ts +++ b/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/redux/validations/vm-settings-tab-validation.ts @@ -44,7 +44,7 @@ import { vmSettingsOrder } from '../initial-state/vm-settings-tab-initial-state' import { TemplateValidations } from '../../../../utils/validations/template/template-validations'; import { combineIntegerValidationResults } from '../../../../utils/validations/template/utils'; import { getValidationUpdate } from './utils'; -import { getTemplateValidations } from './utils/templates-validations'; +import { getTemplateValidations } from '../../selectors/template'; const validateVm: VmSettingsValidator = (field, options) => { const { getState, id } = options; @@ -103,8 +103,10 @@ const memoryValidation: VmSettingsValidator = (field, options): ValidationObject if (memValueGB == null || memValueGB === '') { return null; } + const { id, getState } = options; + const state = getState(); const memValueBytes = memValueGB * 1024 ** 3; - const validations = getTemplateValidations(options); + const validations = getTemplateValidations(state, id); if (validations.length === 0) { validations.push(new TemplateValidations()); // add empty validation for positive integer if it is missing one } diff --git a/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/redux/validations/utils/templates-validations.ts b/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/selectors/template.ts similarity index 59% rename from frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/redux/validations/utils/templates-validations.ts rename to frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/selectors/template.ts index c1ba668c8ad..5ece3f0e038 100644 --- a/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/redux/validations/utils/templates-validations.ts +++ b/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/selectors/template.ts @@ -1,26 +1,19 @@ -import { UpdateOptions } from '../../types'; -import { - iGetVmSettingAttribute, - iGetVmSettingValue, -} from '../../../selectors/immutable/vm-settings'; -import { iGetLoadedCommonData } from '../../../selectors/immutable/selectors'; -import { VMSettingsField, VMWizardProps } from '../../../types'; -import { iGetTemplateValidations } from '../../../../../selectors/immutable/template/selectors'; +import { iGetTemplateValidations } from '../../../selectors/immutable/template/selectors'; +import { TemplateValidations } from '../../../utils/validations/template/template-validations'; import { iGetRelevantTemplate, iGetRelevantTemplates, -} from '../../../../../selectors/immutable/template/combined'; -import { TemplateValidations } from '../../../../../utils/validations/template/template-validations'; +} from '../../../selectors/immutable/template/combined'; +import { VMSettingsField, VMWizardProps } from '../types'; +import { iGetLoadedCommonData } from './immutable/selectors'; +import { iGetVmSettingAttribute, iGetVmSettingValue } from './immutable/vm-settings'; const getValidationsFromTemplates = (templates): TemplateValidations[] => templates.map( (relevantTemplate) => new TemplateValidations(iGetTemplateValidations(relevantTemplate)), ); -export const getTemplateValidations = (options: UpdateOptions): TemplateValidations[] => { - const { getState, id } = options; - const state = getState(); - +export const getTemplateValidations = (state, id: string): TemplateValidations[] => { const userTemplateName = iGetVmSettingValue(state, id, VMSettingsField.USER_TEMPLATE); const os = iGetVmSettingAttribute(state, id, VMSettingsField.OPERATING_SYSTEM); const flavor = iGetVmSettingAttribute(state, id, VMSettingsField.FLAVOR); @@ -54,3 +47,14 @@ export const getTemplateValidations = (options: UpdateOptions): TemplateValidati return getValidationsFromTemplates(relevantTemplates.toArray()); }; + +export const getTemplateValidation = (state, id: string): TemplateValidations => { + const templateValidations = getTemplateValidations(state, id); + if (templateValidations && templateValidations.length > 0) { + templateValidations.length > 1 && + console.warn('WARNING: getTemplateValidation: multiple template validations detected!'); + return templateValidations[0]; + } + + return null; +}; diff --git a/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/storage-tab/vm-wizard-storage-modal-enhanced.tsx b/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/storage-tab/vm-wizard-storage-modal-enhanced.tsx index 8a427e49cd3..54e2984d5dd 100644 --- a/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/storage-tab/vm-wizard-storage-modal-enhanced.tsx +++ b/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/storage-tab/vm-wizard-storage-modal-enhanced.tsx @@ -24,6 +24,9 @@ import { DataVolumeWrapper } from '../../../../k8s/wrapper/vm/data-volume-wrappe import { DiskModal } from '../../../modals/disk-modal'; import { VM_TEMPLATE_NAME_PARAMETER } from '../../../../constants/vm-templates'; import { PersistentVolumeClaimWrapper } from '../../../../k8s/wrapper/vm/persistent-volume-claim-wrapper'; +import { TemplateValidations } from '../../../../utils/validations/template/template-validations'; +import { DiskBus } from '../../../../constants/vm/storage/disk-bus'; +import { getTemplateValidation } from '../../selectors/template'; const VMWizardStorageModal: React.FC = (props) => { const { @@ -34,6 +37,7 @@ const VMWizardStorageModal: React.FC = (props) => { useProjects, addUpdateStorage, storages, + templateValidations, ...restProps } = props; const { @@ -80,6 +84,10 @@ const VMWizardStorageModal: React.FC = (props) => { }, ]; + const allowedBusses: Set = ( + templateValidations || new TemplateValidations() + ).getAllowedBusses(); + return ( = (props) => { onNamespaceChanged={(n) => setNamespace(n)} usedDiskNames={usedDiskNames} usedPVCNames={usedPVCNames} + allowedBusses={allowedBusses} disk={diskWrapper} volume={volumeWrapper} dataVolume={dataVolumeWrapper} @@ -139,6 +148,7 @@ type VMWizardStorageModalProps = ModalComponentProps & { isCreateTemplate: boolean; storages: VMWizardStorageWithWrappers[]; addUpdateStorage: (storage: VMWizardStorage) => void; + templateValidations: TemplateValidations; }; const stateToProps = (state, { wizardReduxID }) => { @@ -148,6 +158,7 @@ const stateToProps = (state, { wizardReduxID }) => { namespace: iGetCommonData(state, wizardReduxID, VMWizardProps.activeNamespace), isCreateTemplate: iGetCommonData(state, wizardReduxID, VMWizardProps.isCreateTemplate), storages: getStoragesWithWrappers(state, wizardReduxID), + templateValidations: getTemplateValidation(state, wizardReduxID), }; }; diff --git a/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/storage-tab/vm-wizard-storage-row.tsx b/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/storage-tab/vm-wizard-storage-row.tsx index d3b06e52b1e..dee44afff57 100644 --- a/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/storage-tab/vm-wizard-storage-row.tsx +++ b/frontend/packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/storage-tab/vm-wizard-storage-row.tsx @@ -76,6 +76,7 @@ export const VmWizardStorageRow: React.FC = ({ validation={{ name: validations.name || validations.url || validations.container || validations.pvc, size: validations.size, + diskInterface: validations.diskInterface, }} columnClasses={columnClasses} index={index} diff --git a/frontend/packages/kubevirt-plugin/src/components/modals/disk-modal/disk-modal.tsx b/frontend/packages/kubevirt-plugin/src/components/modals/disk-modal/disk-modal.tsx index bf15d7e716b..b49acdbfe27 100644 --- a/frontend/packages/kubevirt-plugin/src/components/modals/disk-modal/disk-modal.tsx +++ b/frontend/packages/kubevirt-plugin/src/components/modals/disk-modal/disk-modal.tsx @@ -69,11 +69,13 @@ export const DiskModal = withHandlePromise((props: DiskModalProps) => { handlePromise, close, cancel, + allowedBusses = new Set(DiskBus.getAll()), } = props; const asId = prefixedID.bind(null, 'disk'); const disk = props.disk || DiskWrapper.EMPTY; const volume = props.volume || VolumeWrapper.EMPTY; const dataVolume = props.dataVolume || DataVolumeWrapper.EMPTY; + const validAllowedBusses = allowedBusses || new Set(DiskBus.getAll()); const combinedDisk = new CombinedDisk({ diskWrapper: disk, @@ -102,7 +104,12 @@ export const DiskModal = withHandlePromise((props: DiskModalProps) => { disk.getName() || getSequenceName('disk', usedDiskNames), ); const [bus, setBus] = React.useState( - disk.getDiskBus() || (isEditing ? null : DiskBus.VIRTIO), + disk.getDiskBus() || + (isEditing + ? null + : validAllowedBusses.has(DiskBus.VIRTIO) + ? DiskBus.VIRTIO + : [...validAllowedBusses][0]), ); const [storageClassName, setStorageClassName] = React.useState( combinedDisk.getStorageClassName(), @@ -170,6 +177,7 @@ export const DiskModal = withHandlePromise((props: DiskModalProps) => { size: sizeValidation, container: containerValidation, pvc: pvcValidation, + diskInterface: busValidation, url: urlValidation, }, isValid, @@ -177,6 +185,7 @@ export const DiskModal = withHandlePromise((props: DiskModalProps) => { } = validateDisk(resultDisk, resultVolume, resultDataVolume, resultPersistentVolumeClaim, { usedDiskNames, usedPVCNames, + allowedBusses, }); const [showUIError, setShowUIError] = useShowErrorToggler(false, isValid, isValid); @@ -370,17 +379,30 @@ export const DiskModal = withHandlePromise((props: DiskModalProps) => { /> )} - + setBus(DiskBus.fromString(diskBus)), [ setBus, ])} + isValid={!isValidationError(busValidation)} value={asFormSelectValue(bus)} id={asId('interface')} isDisabled={inProgress} > - {DiskBus.getAll().map((b) => ( + {!validAllowedBusses.has(bus) && ( + + )} + {[...validAllowedBusses].map((b) => ( ))} @@ -444,6 +466,7 @@ export type DiskModalProps = { vmNamespace: string; namespace: string; onNamespaceChanged: (namespace: string) => void; + allowedBusses?: Set; usedDiskNames: Set; usedPVCNames: Set; } & ModalComponentProps & diff --git a/frontend/packages/kubevirt-plugin/src/utils/validations/common.ts b/frontend/packages/kubevirt-plugin/src/utils/validations/common.ts index 6ffe2a8e0dc..acaf2c5f22b 100644 --- a/frontend/packages/kubevirt-plugin/src/utils/validations/common.ts +++ b/frontend/packages/kubevirt-plugin/src/utils/validations/common.ts @@ -7,9 +7,11 @@ import { validateEmptyValue, ValidationErrorType, ValidationObject, + joinGrammaticallyListOfItems, } from '@console/shared'; import { parseURL } from '../url'; import { END_WHITESPACE_ERROR, START_WHITESPACE_ERROR, URL_INVALID_ERROR } from './strings'; +import { DiskBus } from '../../constants/vm/storage/disk-bus'; export const isValidationError = (validationObject: ValidationObject) => !!validationObject && validationObject.type === ValidationErrorType.Error; @@ -81,3 +83,15 @@ export const validateURL = ( return parseURL(value) ? null : asValidationObject(addMissingSubject(URL_INVALID_ERROR, subject)); }; + +export const validateBus = (value: DiskBus, allowedBusses: Set): ValidationObject => { + if (allowedBusses && !allowedBusses.has(value)) { + return asValidationObject( + `Invalid interface type. Valid types are: ${joinGrammaticallyListOfItems( + [...allowedBusses].map((b) => b.toString()), + )}`, + ValidationErrorType.Error, + ); + } + return null; +}; diff --git a/frontend/packages/kubevirt-plugin/src/utils/validations/template/interval-validation-result.ts b/frontend/packages/kubevirt-plugin/src/utils/validations/template/interval-validation-result.ts index c566ba4e034..1580a6a25c4 100644 --- a/frontend/packages/kubevirt-plugin/src/utils/validations/template/interval-validation-result.ts +++ b/frontend/packages/kubevirt-plugin/src/utils/validations/template/interval-validation-result.ts @@ -44,7 +44,7 @@ export class MemoryIntervalValidationResult extends IntervalValidationResult { }`; } if (Number.isFinite(this.max)) { - return `Memory ${verb} be ${this.isMaxInclusive ? 'at most' : 'bellow'} ${ + return `Memory ${verb} be ${this.isMaxInclusive ? 'at most' : 'below'} ${ humanizeBinaryBytes(this.max).string }`; } diff --git a/frontend/packages/kubevirt-plugin/src/utils/validations/template/template-validations.ts b/frontend/packages/kubevirt-plugin/src/utils/validations/template/template-validations.ts index b648b72e37f..903ba5aa6d8 100644 --- a/frontend/packages/kubevirt-plugin/src/utils/validations/template/template-validations.ts +++ b/frontend/packages/kubevirt-plugin/src/utils/validations/template/template-validations.ts @@ -1,7 +1,7 @@ /* eslint-disable lines-between-class-members */ import * as _ from 'lodash'; import { ValidationErrorType } from '@console/shared/src'; -import { ValueEnum } from '../../../constants'; +import { ValueEnum, DiskBus } from '../../../constants'; import { CommonTemplatesValidation } from '../../../types/template'; import { IntervalValidationResult, @@ -14,6 +14,7 @@ export class ValidationJSONPath extends ValueEnum { static readonly MEMORY = new ValidationJSONPath( 'jsonpath::.spec.domain.resources.requests.memory', ); + static readonly BUS = new ValidationJSONPath('jsonpath::.spec.domain.devices.disks[*].disk.bus'); } export class TemplateValidations { @@ -110,6 +111,28 @@ export class TemplateValidations { }); }; + getAllowedBusses = (): Set => { + const allowedBusses = this.getAllowedEnumValues( + ValidationJSONPath.BUS, + ValidationErrorType.Error, + ).map(DiskBus.fromString); + + return new Set(allowedBusses.length === 0 ? DiskBus.getAll() : allowedBusses); + }; + + // Empty array means all values are allowed + private getAllowedEnumValues = ( + jsonPath: ValidationJSONPath, + type: ValidationErrorType, + ): string[] => { + const relevantValidations = this.getRelevantValidations(jsonPath, type); + + return relevantValidations.reduce( + (result: string[], validation: CommonTemplatesValidation) => result.concat(validation.values), + [], + ); + }; + private getRelevantValidations = (jsonPath: ValidationJSONPath, type: ValidationErrorType) => { return this.validations.filter( (validation: CommonTemplatesValidation) => diff --git a/frontend/packages/kubevirt-plugin/src/utils/validations/vm/disk.ts b/frontend/packages/kubevirt-plugin/src/utils/validations/vm/disk.ts index 95b3cf9ef6d..13c57188c10 100644 --- a/frontend/packages/kubevirt-plugin/src/utils/validations/vm/disk.ts +++ b/frontend/packages/kubevirt-plugin/src/utils/validations/vm/disk.ts @@ -6,7 +6,7 @@ import { ValidationErrorType, ValidationObject, } from '@console/shared'; -import { validateTrim, validateURL } from '../common'; +import { validateTrim, validateURL, validateBus } from '../common'; import { DiskWrapper } from '../../../k8s/wrapper/vm/disk-wrapper'; import { VolumeWrapper } from '../../../k8s/wrapper/vm/volume-wrapper'; import { DataVolumeWrapper } from '../../../k8s/wrapper/vm/data-volume-wrapper'; @@ -14,6 +14,8 @@ import { POSITIVE_SIZE_ERROR } from '../strings'; import { StorageUISource } from '../../../components/modals/disk-modal/storage-ui-source'; import { CombinedDisk } from '../../../k8s/wrapper/vm/combined-disk'; import { PersistentVolumeClaimWrapper } from '../../../k8s/wrapper/vm/persistent-volume-claim-wrapper'; +import { DiskBus } from '../../../constants/vm/storage/disk-bus'; +import { DiskType } from '../../../constants/vm/storage/disk-type'; const validateDiskName = (name: string, usedDiskNames: Set): ValidationObject => { let validation = validateDNS1123SubdomainValue(name); @@ -51,9 +53,11 @@ export const validateDisk = ( { usedDiskNames, usedPVCNames, + allowedBusses, }: { usedDiskNames?: Set; usedPVCNames?: Set; + allowedBusses: Set; }, ): UIDiskValidation => { const validations = { @@ -61,6 +65,7 @@ export const validateDisk = ( size: null, url: null, container: null, + diskInterface: null, pvc: null, }; let hasAllRequiredFilled = disk && disk.getName(); @@ -129,8 +134,13 @@ export const validateDisk = ( addRequired(pvcName); validations.pvc = validatePVCName(pvcName, usedPVCNames); } - } + // TODO: implement CDROM disk bus validation + if (disk.getType() === DiskType.DISK) { + addRequired(disk.getDiskBus()); + validations.diskInterface = validateBus(disk.getDiskBus(), allowedBusses); + } + } return { validations, hasAllRequiredFilled: !!hasAllRequiredFilled, @@ -144,6 +154,7 @@ export type UIDiskValidation = { size?: ValidationObject; url?: ValidationObject; container?: ValidationObject; + diskInterface?: ValidationObject; pvc?: ValidationObject; }; isValid: boolean;