From 8a19ee65d09882741d338731f765c7f818c2c7fc Mon Sep 17 00:00:00 2001 From: suomiy Date: Wed, 15 May 2019 09:04:35 +0200 Subject: [PATCH] Display error on duplicate VM name in wizard (#449) --- .../Wizard/CreateVmWizard/CreateVmWizard.js | 12 +- .../Wizard/CreateVmWizard/constants.js | 3 + .../fixtures/CreateVmWizard.fixture.js | 4 + .../tests/CreateVmWizard.test.js | 3 +- .../CreateVmWizard/validations/utils.js | 4 +- .../CreateVmWizard/validations/validators.js | 4 +- .../validations/vmSettingsTabValidation.js | 13 +- src/tests/mocks/vm/vm_validation.mock.js | 169 ++++++++++++++++++ src/utils/strings.js | 2 +- src/utils/tests/validation.test.js | 19 ++ src/utils/validations.js | 18 +- 11 files changed, 231 insertions(+), 20 deletions(-) create mode 100644 src/tests/mocks/vm/vm_validation.mock.js diff --git a/src/components/Wizard/CreateVmWizard/CreateVmWizard.js b/src/components/Wizard/CreateVmWizard/CreateVmWizard.js index 74f691802..e20d16466 100644 --- a/src/components/Wizard/CreateVmWizard/CreateVmWizard.js +++ b/src/components/Wizard/CreateVmWizard/CreateVmWizard.js @@ -56,7 +56,7 @@ const getUpdatedValidatedState = (prevProps, prevState, props, state, extra) => const validatedStepData = ALL_TAB_KEYS.reduce((resultStepData, tabKey) => { const { value, valid } = newState.stepData[tabKey]; - resultStepData[tabKey] = validateTabData(tabKey, value, valid); + resultStepData[tabKey] = validateTabData(tabKey, value, valid, props); return resultStepData; }, {}); @@ -84,6 +84,7 @@ export class CreateVmWizard extends React.Component { this.state = getUpdatedValidatedState(null, null, props, initialState, { // eslint-disable-next-line no-console safeSetState: () => console.warn('setState not supported when initializing'), + virtualMachines: this.props.virtualMachines, }); } @@ -104,6 +105,7 @@ export class CreateVmWizard extends React.Component { componentDidUpdate(prevProps, prevState, snapshot) { const newState = getUpdatedValidatedState(prevProps, prevState, this.props, this.state, { safeSetState: this.safeSetState, + virtualMachines: this.props.virtualMachines, }); if (this.state !== newState) { @@ -117,7 +119,7 @@ export class CreateVmWizard extends React.Component { lastStepReached = () => this.state.activeStepIndex === this.getLastStepIndex(); onStepDataChanged = (tabKey, value, valid, lockStep = false) => { - const validatedTabData = validateTabData(tabKey, value, valid); + const validatedTabData = validateTabData(tabKey, value, valid, this.props); this.safeSetState(state => ({ stepData: { ...state.stepData, @@ -185,9 +187,9 @@ export class CreateVmWizard extends React.Component { key: VM_SETTINGS_TAB_KEY, onCloseWizard: onCloseVmSettings, render: () => { - const { namespaces, templates, dataVolumes, WithResources } = this.props; + const { namespaces, templates, dataVolumes, virtualMachines, WithResources } = this.props; - const loadingData = { namespaces, templates, dataVolumes }; + const loadingData = { namespaces, templates, dataVolumes, virtualMachines }; const vmSettings = getVmSettings(this.state); return ( @@ -305,6 +307,7 @@ export class CreateVmWizard extends React.Component { CreateVmWizard.defaultProps = { templates: null, namespaces: null, + virtualMachines: null, selectedNamespace: null, networkConfigs: null, persistentVolumeClaims: null, @@ -322,6 +325,7 @@ CreateVmWizard.propTypes = { onHide: PropTypes.func.isRequired, templates: PropTypes.array, namespaces: PropTypes.array, + virtualMachines: PropTypes.array, selectedNamespace: PropTypes.object, networkConfigs: PropTypes.array, persistentVolumeClaims: PropTypes.array, diff --git a/src/components/Wizard/CreateVmWizard/constants.js b/src/components/Wizard/CreateVmWizard/constants.js index 99f9ba96a..6267beca0 100644 --- a/src/components/Wizard/CreateVmWizard/constants.js +++ b/src/components/Wizard/CreateVmWizard/constants.js @@ -43,3 +43,6 @@ export const STORAGE_TYPE_CONTAINER = 'container'; export const DATA_VOLUME_SOURCE_BLANK = 'datavolume-blank'; export const DATA_VOLUME_SOURCE_URL = 'datavolume-url'; export const DATA_VOLUME_SOURCE_PVC = 'datavolume-pvc'; + +// Additional resource keys +export const VIRTUAL_MACHINES_KEY = 'virtualMachines'; diff --git a/src/components/Wizard/CreateVmWizard/fixtures/CreateVmWizard.fixture.js b/src/components/Wizard/CreateVmWizard/fixtures/CreateVmWizard.fixture.js index 7ccd0c778..e67a8cfd8 100644 --- a/src/components/Wizard/CreateVmWizard/fixtures/CreateVmWizard.fixture.js +++ b/src/components/Wizard/CreateVmWizard/fixtures/CreateVmWizard.fixture.js @@ -64,6 +64,7 @@ export default [ storageClasses, units, dataVolumes: [urlTemplateDataVolume], + virtualMachines: [], }, }, { @@ -80,6 +81,7 @@ export default [ storageClasses, units, dataVolumes: [urlTemplateDataVolume], + virtualMachines: [], }, }, { @@ -95,6 +97,7 @@ export default [ storageClasses: null, units, dataVolumes: null, + virtualMachines: null, }, }, { @@ -111,6 +114,7 @@ export default [ units, createTemplate: true, dataVolumes: [urlTemplateDataVolume], + virtualMachines: [], }, }, ]; diff --git a/src/components/Wizard/CreateVmWizard/tests/CreateVmWizard.test.js b/src/components/Wizard/CreateVmWizard/tests/CreateVmWizard.test.js index 67581df9b..04807d4a6 100644 --- a/src/components/Wizard/CreateVmWizard/tests/CreateVmWizard.test.js +++ b/src/components/Wizard/CreateVmWizard/tests/CreateVmWizard.test.js @@ -109,7 +109,8 @@ const testWalkThrough = (template = false, createText = CREATE_VM, templatesDrop const namespaces = [...CreateVmWizardFixutre[0].props.namespaces]; const templates = [...CreateVmWizardFixutre[0].props.templates]; const dataVolumes = [...CreateVmWizardFixutre[0].props.dataVolumes]; - component.setProps({ namespaces, templates, dataVolumes }); + const virtualMachines = []; + component.setProps({ namespaces, templates, dataVolumes, virtualMachines }); expect(component.find(Loading)).toHaveLength(0); expect(component.find(VmSettingsTab)).toHaveLength(1); diff --git a/src/components/Wizard/CreateVmWizard/validations/utils.js b/src/components/Wizard/CreateVmWizard/validations/utils.js index d898c6f1d..cf5b6128b 100644 --- a/src/components/Wizard/CreateVmWizard/validations/utils.js +++ b/src/components/Wizard/CreateVmWizard/validations/utils.js @@ -1,8 +1,8 @@ import { get } from 'lodash'; -export const asGenericFieldValidator = (customValidator, getFieldTitle) => (key, vmSettings) => { +export const asGenericFieldValidator = (customValidator, getFieldTitle) => (key, vmSettings, additionalResources) => { const field = vmSettings[key] || {}; - const { validation, ...rest } = customValidator && customValidator(field.value, vmSettings); + const { validation, ...rest } = customValidator && customValidator(field.value, vmSettings, additionalResources); let val = validation; // next step is disabled by isValid so empty errors do not need to be shown diff --git a/src/components/Wizard/CreateVmWizard/validations/validators.js b/src/components/Wizard/CreateVmWizard/validations/validators.js index 008c06a8d..b850214bf 100644 --- a/src/components/Wizard/CreateVmWizard/validations/validators.js +++ b/src/components/Wizard/CreateVmWizard/validations/validators.js @@ -9,12 +9,12 @@ const isTabValidResolver = { [VM_SETTINGS_TAB_KEY]: isVmSettingsTabValid, }; -export const validateTabData = (tabKey, data, valid) => { +export const validateTabData = (tabKey, data, valid, props) => { const dataValidator = validateTabResolver[tabKey]; const tabValidator = isTabValidResolver[tabKey]; if (dataValidator) { - data = dataValidator(data); + data = dataValidator(data, props); } if (tabValidator) { diff --git a/src/components/Wizard/CreateVmWizard/validations/vmSettingsTabValidation.js b/src/components/Wizard/CreateVmWizard/validations/vmSettingsTabValidation.js index 3adf66995..c5c51e184 100644 --- a/src/components/Wizard/CreateVmWizard/validations/vmSettingsTabValidation.js +++ b/src/components/Wizard/CreateVmWizard/validations/vmSettingsTabValidation.js @@ -1,12 +1,7 @@ import { get } from 'lodash'; import { isFieldRequired } from '../utils/vmSettingsTabUtils'; -import { - getValidationObject, - validateContainer, - validateDNS1123SubdomainValue, - validateURL, -} from '../../../../utils/validations'; +import { getValidationObject, validateContainer, validateVmName, validateURL } from '../../../../utils/validations'; import { objectMerge } from '../../../../utils/utils'; import { settingsValue } from '../../../../k8s/selectors'; import { PROVISION_SOURCE_IMPORT, VALIDATION_ERROR_TYPE } from '../../../../constants'; @@ -41,19 +36,19 @@ const validateProviderDropdown = (key, vmSettings) => { const asVmSettingsValidator = validator => asGenericFieldValidator(asUpdateValidator(validator), getFieldTitle); const validateResolver = { - [NAME_KEY]: asVmSettingsValidator(validateDNS1123SubdomainValue), + [NAME_KEY]: asVmSettingsValidator(validateVmName), [CONTAINER_IMAGE_KEY]: asVmSettingsValidator(validateContainer), [IMAGE_URL_KEY]: asVmSettingsValidator(validateURL), [PROVIDER_KEY]: validateProviderDropdown, }; -export const validateVmSettings = vmSettings => { +export const validateVmSettings = (vmSettings, additionalResources) => { const update = {}; Object.keys(vmSettings).forEach(key => { const validator = validateResolver[key]; if (validator) { - update[key] = validator(key, vmSettings); + update[key] = validator(key, vmSettings, additionalResources); } }); diff --git a/src/tests/mocks/vm/vm_validation.mock.js b/src/tests/mocks/vm/vm_validation.mock.js new file mode 100644 index 000000000..6605516d0 --- /dev/null +++ b/src/tests/mocks/vm/vm_validation.mock.js @@ -0,0 +1,169 @@ +export const vm1 = { + apiVersion: 'kubevirt.io/v1alpha3', + kind: 'VirtualMachine', + metadata: { + clusterName: '', + creationTimestamp: '2018-11-06T14:32:07Z', + generation: 1, + name: 'vm1', + namespace: 'test-namespace', + resourceVersion: '10390764', + selfLink: '/apis/kubevirt.io/v1alpha3/namespaces/default/virtualmachines/vm1', + uid: 'bcc1d0b1-e1d0-11e8-82b4-54ee7586b9c4', + }, + status: { + created: true, + ready: true, + }, + spec: { + dataVolumeTemplates: [ + { + metadata: { + name: 'dv-template', + }, + spec: { + source: { + pvc: { + name: 'fooname', + namespace: 'foonamespace', + }, + }, + pvc: { + accessModes: ['ReadWriteOnce'], + resources: { + requests: { + storage: '1G', + }, + }, + }, + }, + }, + ], + running: true, + template: { + spec: { + domain: { + cpu: { cores: 2 }, + devices: { + disks: [ + { + disk: { + bus: 'virtio', + }, + name: 'rootdisk', + }, + ], + interfaces: [ + { + bridge: {}, + name: 'eth0', + }, + ], + rng: {}, + }, + resources: { + requests: { memory: '2G' }, + }, + }, + networks: [ + { + name: 'eth0', + pod: {}, + }, + ], + terminationGracePeriodSeconds: 0, + volumes: [ + { + name: 'rootdisk', + containerDisk: { image: 'kubevirt/cirros-registry-disk-demo' }, + }, + ], + }, + }, + }, +}; + +export const vm2 = { + apiVersion: 'kubevirt.io/v1alpha3', + kind: 'VirtualMachine', + metadata: { + clusterName: '', + creationTimestamp: '2018-11-06T14:32:07Z', + generation: 1, + name: 'vm2', + namespace: 'test-namespace', + resourceVersion: '10390764', + selfLink: '/apis/kubevirt.io/v1alpha3/namespaces/default/virtualmachines/vm2', + uid: 'bcc1d0b1-e1d0-11e8-82b4-54ee7586b9c5', + }, + status: { + created: true, + ready: true, + }, + spec: { + dataVolumeTemplates: [ + { + metadata: { + name: 'dv-template', + }, + spec: { + source: { + pvc: { + name: 'fooname', + namespace: 'foonamespace', + }, + }, + pvc: { + accessModes: ['ReadWriteOnce'], + resources: { + requests: { + storage: '1G', + }, + }, + }, + }, + }, + ], + running: true, + template: { + spec: { + domain: { + cpu: { cores: 2 }, + devices: { + disks: [ + { + disk: { + bus: 'virtio', + }, + name: 'rootdisk', + }, + ], + interfaces: [ + { + bridge: {}, + name: 'eth0', + }, + ], + rng: {}, + }, + resources: { + requests: { memory: '2G' }, + }, + }, + networks: [ + { + name: 'eth0', + pod: {}, + }, + ], + terminationGracePeriodSeconds: 0, + volumes: [ + { + name: 'rootdisk', + containerDisk: { image: 'kubevirt/cirros-registry-disk-demo' }, + }, + ], + }, + }, + }, +}; diff --git a/src/utils/strings.js b/src/utils/strings.js index ba240ecd7..849a3c631 100644 --- a/src/utils/strings.js +++ b/src/utils/strings.js @@ -12,7 +12,7 @@ export const DNS1123_UPPERCASE_ERROR = 'cannot contain uppercase letters'; export const URL_INVALID_ERROR = 'has to be a valid URL'; -export const VIRTUAL_MACHINE_EXISTS = `is already used by another Virtual Machine`; +export const VIRTUAL_MACHINE_EXISTS = `is already used by another virtual machine`; // export const VMWARE_URL_ERROR = 'vCenter URL is incorrectly formatted. Example: https://host:port/'; export const ERROR = 'Error'; diff --git a/src/utils/tests/validation.test.js b/src/utils/tests/validation.test.js index db09378ff..419c1b7e3 100644 --- a/src/utils/tests/validation.test.js +++ b/src/utils/tests/validation.test.js @@ -5,6 +5,7 @@ import { validateContainer, getValidationObject, validateVmwareURL, + validateVmName, } from '../validations'; import { DNS1123_START_ERROR, @@ -16,7 +17,11 @@ import { URL_INVALID_ERROR, END_WHITESPACE_ERROR, START_WHITESPACE_ERROR, + VIRTUAL_MACHINE_EXISTS, } from '../strings'; +import { vm1, vm2 } from '../../tests/mocks/vm/vm_validation.mock'; +import { validVmSettings } from '../../components/Wizard/CreateVmWizard/fixtures/VmSettingsTab.fixture'; +import { NAMESPACE_KEY } from '../../components/Wizard/CreateVmWizard/constants'; const validatesEmpty = validateFunction => { expect(validateFunction('')).toEqual(getValidationObject(EMPTY_ERROR)); @@ -125,3 +130,17 @@ describe('validation.js - validateVmwareURL', () => { expect(validateVmwareURL('http://hello.com ')).toEqual(getValidationObject(END_WHITESPACE_ERROR)); }); }); + +describe('validation.js - validateVmName', () => { + const props = { virtualMachines: [vm1, vm2] }; + const vmSettings = validVmSettings; + vmSettings[NAMESPACE_KEY].value = 'test-namespace'; + + it('handles unique name', () => { + expect(validateVmName('vm3', vmSettings, props)).toBeNull(); + }); + + it('handles duplicate name', () => { + expect(validateVmName('vm1', vmSettings, props)).toEqual(getValidationObject(VIRTUAL_MACHINE_EXISTS)); + }); +}); diff --git a/src/utils/validations.js b/src/utils/validations.js index 06c44bd3c..a2f53a93d 100644 --- a/src/utils/validations.js +++ b/src/utils/validations.js @@ -1,5 +1,5 @@ /* eslint-disable no-new */ -import { trimStart, trimEnd } from 'lodash'; +import { get, trimStart, trimEnd } from 'lodash'; import { DNS1123_START_ERROR, @@ -11,11 +11,14 @@ import { URL_INVALID_ERROR, START_WHITESPACE_ERROR, END_WHITESPACE_ERROR, + VIRTUAL_MACHINE_EXISTS, } from './strings'; import { parseUrl } from './utils'; import { VALIDATION_ERROR_TYPE } from '../constants'; +import { getName, getNamespace } from '../selectors'; +import { NAMESPACE_KEY, VIRTUAL_MACHINES_KEY } from '../components/Wizard/CreateVmWizard/constants'; export const isPositiveNumber = value => value && value.match(/^[1-9]\d*$/); @@ -54,6 +57,19 @@ export const validateDNS1123SubdomainValue = value => { return null; }; +const vmAlreadyExists = (name, namespace, vms) => { + const exists = vms && vms.some(vm => getName(vm) === name && getNamespace(vm) === namespace); + return exists ? getValidationObject(VIRTUAL_MACHINE_EXISTS) : null; +}; + +export const validateVmName = (value, vmSettings, props) => { + const namespace = get(vmSettings, `${NAMESPACE_KEY}.value`); + const dnsValidation = validateDNS1123SubdomainValue(value); + return dnsValidation && dnsValidation.type === VALIDATION_ERROR_TYPE + ? dnsValidation + : vmAlreadyExists(value, namespace, props[VIRTUAL_MACHINES_KEY]); +}; + export const validateURL = value => { if (!value) { return getValidationObject(EMPTY_ERROR);