Skip to content

Commit 9ee00c7

Browse files
momesginMo Mesgin
andauthored
Validation for duplicate pool names (#17512)
* add duplicate pool names validation * fix case sensitive validation issue + unit tests * fix re-rendering issue on the vertical tabs * fix not using translation key * show duplicate pool name error without requiring field interaction --------- Co-authored-by: Mo Mesgin <mmesgin@Mos-M2-MacBook-Pro.local>
1 parent 9a37eea commit 9ee00c7

5 files changed

Lines changed: 120 additions & 3 deletions

File tree

shell/assets/translations/en-us.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2341,7 +2341,9 @@ cluster:
23412341
etcdControlPlaneWarning: We don't recommend using Autoscaler with machine pools that have the etcd or Control Plane roles. You can modify the yaml if you want to override the current value.
23422342
name:
23432343
label: Pool Name
2344+
notNamed: (Not Named)
23442345
placeholder: A random one will be generated by default
2346+
unique: Pool names should be unique
23452347
nodeTotals:
23462348
label:
23472349
controlPlane: '{count} Control Plane'
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
import { shallowMount } from '@vue/test-utils';
2+
import MachinePool from '@shell/edit/provisioning.cattle.io.cluster/tabs/MachinePool.vue';
3+
4+
const TRANSLATION_KEY = '%cluster.machinePool.name.unique%';
5+
6+
function createPool(name: string, { remove = false } = {}) {
7+
return {
8+
id: `pool-${ name }`,
9+
remove,
10+
create: false,
11+
update: true,
12+
pool: {
13+
name,
14+
etcdRole: false,
15+
controlPlaneRole: false,
16+
workerRole: true,
17+
quantity: 1,
18+
},
19+
config: null,
20+
};
21+
}
22+
23+
function mountMachinePool(currentPool: ReturnType<typeof createPool>, allPools: ReturnType<typeof createPool>[]) {
24+
return shallowMount(MachinePool, {
25+
props: {
26+
value: currentPool,
27+
mode: 'create',
28+
provider: 'custom',
29+
idx: 0,
30+
machinePools: allPools,
31+
poolId: currentPool.id,
32+
poolCreateMode: true,
33+
},
34+
global: {
35+
mocks: {
36+
$store: {
37+
getters: {
38+
'i18n/t': (key: string) => key,
39+
'i18n/exists': () => false,
40+
'type-map/hasCustomMachineConfigComponent': () => false,
41+
'type-map/importMachineConfig': () => null,
42+
'features/get': () => false,
43+
},
44+
dispatch: jest.fn(),
45+
},
46+
},
47+
stubs: {
48+
LabeledInput: true,
49+
Checkbox: true,
50+
Taints: true,
51+
KeyValue: true,
52+
AdvancedSection: true,
53+
Banner: true,
54+
UnitInput: true,
55+
},
56+
},
57+
});
58+
}
59+
60+
describe('component: MachinePool', () => {
61+
describe('uniquePoolName validation', () => {
62+
it('should return undefined when the name is empty', () => {
63+
const pool = createPool('');
64+
const wrapper = mountMachinePool(pool, [pool]);
65+
66+
expect(wrapper.vm.fvExtraRules.uniquePoolName('')).toBeUndefined();
67+
});
68+
69+
it('should return undefined when the pool name is unique', () => {
70+
const pool1 = createPool('pool1');
71+
const pool2 = createPool('pool2');
72+
const wrapper = mountMachinePool(pool1, [pool1, pool2]);
73+
74+
expect(wrapper.vm.fvExtraRules.uniquePoolName('pool1')).toBeUndefined();
75+
});
76+
77+
it('should return an error message when the pool name is duplicated', () => {
78+
const pool1 = createPool('same-name');
79+
const pool2 = createPool('same-name');
80+
const wrapper = mountMachinePool(pool1, [pool1, pool2]);
81+
82+
expect(wrapper.vm.fvExtraRules.uniquePoolName('same-name')).toStrictEqual(TRANSLATION_KEY);
83+
});
84+
85+
it('should ignore pools marked for removal', () => {
86+
const pool1 = createPool('same-name');
87+
const pool2 = createPool('same-name', { remove: true });
88+
const wrapper = mountMachinePool(pool1, [pool1, pool2]);
89+
90+
expect(wrapper.vm.fvExtraRules.uniquePoolName('same-name')).toBeUndefined();
91+
});
92+
93+
it.each([
94+
['Pool1', 'pool1'],
95+
['POOL', 'pool'],
96+
])('should flag names that differ only by case as duplicates (%s vs %s)', (nameA, nameB) => {
97+
const pool1 = createPool(nameA);
98+
const pool2 = createPool(nameB);
99+
const wrapper = mountMachinePool(pool1, [pool1, pool2]);
100+
101+
expect(wrapper.vm.fvExtraRules.uniquePoolName(nameA)).toStrictEqual(TRANSLATION_KEY);
102+
});
103+
});
104+
});

shell/edit/provisioning.cattle.io.cluster/rke2.vue

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2518,14 +2518,14 @@ export default {
25182518
>
25192519
<template
25202520
v-for="(obj, idx) in machinePools"
2521-
:key="idx"
2521+
:key="obj.id"
25222522
>
25232523
<Tab
25242524
v-if="!obj.remove"
25252525
:key="obj.id"
25262526
:weight="-1 * idx"
25272527
:name="obj.id"
2528-
:label="obj.pool.name || '(Not Named)'"
2528+
:label="obj.pool.name || t('cluster.machinePool.name.notNamed')"
25292529
:show-header="false"
25302530
:error="!machinePoolValidation[obj.id]"
25312531
>

shell/edit/provisioning.cattle.io.cluster/tabs/MachinePool.vue

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,16 @@ export default {
106106
const max = this.value?.pool?.autoscalingMaxSize || 0;
107107
108108
return max - min >= 0 ? undefined : this.t('cluster.machinePool.autoscaler.validation.isAutoscalerMaxGreaterThanMin');
109+
},
110+
uniquePoolName: (name) => {
111+
if (!name) {
112+
return undefined;
113+
}
114+
115+
const otherPools = (this.machinePools || []).filter((p) => !p.remove && p !== this.value);
116+
const isDuplicate = otherPools.some((p) => p.pool.name?.toLowerCase() === name.toLowerCase());
117+
118+
return isDuplicate ? this.t('cluster.machinePool.name.unique') : undefined;
109119
}
110120
}
111121
};
@@ -295,6 +305,7 @@ export default {
295305
:label="t('cluster.machinePool.name.label')"
296306
:required="true"
297307
:disabled="!value.config || !!value.config.id || busy"
308+
:require-dirty="false"
298309
:rules="fvGetAndReportPathRules(MACHINE_POOL_VALIDATION.FIELDS.NAME)"
299310
data-testid="machine-pool-name-input"
300311
/>

shell/utils/validators/machine-pool.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ const RULESETS = [
1212
},
1313
{
1414
path: FIELDS.NAME,
15-
rules: ['required'],
15+
rules: ['required', 'uniquePoolName'],
1616
},
1717
{
1818
path: FIELDS.AUTOSCALER_MIN,

0 commit comments

Comments
 (0)