Skip to content

Commit 40958c2

Browse files
upcoming: [UIE-9515] - Update Firewall Rules Edit & Add Drawer to Support Prefix List Selection (#13138)
* Update Rules Edit/Add drawer to support PrefixLists * Update comment * Add some mocks for prefixlists * Update mock data order * Reset pls state on address change * Feature-flagged add/edit prefixlists * Some clean up * Some changes * More clean up * More clean up * Add code comments and some clean up * Add key and test ids to the pl rows * A small fix * Add unit tests * More clean up * Rename component * Fix one test case * Few fixes * Fix type import * Update mocks for PLs with more possible cases * Some more changes related to sorting * Update mocks * Added changeset: Update Firewall Rules Edit & Add Drawer to Support Prefix List Selection * Add/update comments
1 parent 533c629 commit 40958c2

File tree

13 files changed

+1163
-118
lines changed

13 files changed

+1163
-118
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@linode/manager": Upcoming Features
3+
---
4+
5+
Update Firewall Rules Edit & Add Drawer to Support Prefix List Selection ([#13138](https://github.com/linode/manager/pull/13138))

packages/manager/src/components/MultipleIPInput/MultipleIPInput.tsx

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const useStyles = makeStyles()((theme: Theme) => ({
2626
justifyContent: 'flex-start',
2727
},
2828
paddingLeft: 0,
29-
paddingTop: theme.spacing(1.5),
29+
paddingTop: theme.spacingFunction(12),
3030
},
3131
button: {
3232
'& > span': {
@@ -70,6 +70,12 @@ export interface MultipeIPInputProps {
7070
*/
7171
buttonText?: string;
7272

73+
/**
74+
* Whether the first input field can be removed.
75+
* @default false
76+
*/
77+
canRemoveFirstInput?: boolean;
78+
7379
/**
7480
* Custom CSS class for additional styling.
7581
*/
@@ -155,6 +161,7 @@ export const MultipleIPInput = React.memo((props: MultipeIPInputProps) => {
155161
const {
156162
adjustSpacingForVPCDualStack,
157163
buttonText,
164+
canRemoveFirstInput,
158165
className,
159166
disabled,
160167
error,
@@ -244,8 +251,8 @@ export const MultipleIPInput = React.memo((props: MultipeIPInputProps) => {
244251
<TooltipIcon
245252
status="info"
246253
sxTooltipIcon={{
247-
marginLeft: '-4px',
248-
marginTop: '-15px',
254+
marginTop: '-8px',
255+
padding: '4px',
249256
}}
250257
text={tooltip}
251258
tooltipPosition="right"
@@ -304,7 +311,10 @@ export const MultipleIPInput = React.memo((props: MultipeIPInputProps) => {
304311
* used in DBaaS or for Linode VPC interfaces
305312
*/}
306313
<Grid size={1}>
307-
{(idx > 0 || forDatabaseAccessControls || forVPCIPRanges) && (
314+
{(idx > 0 ||
315+
forDatabaseAccessControls ||
316+
forVPCIPRanges ||
317+
canRemoveFirstInput) && (
308318
<IconButton
309319
aria-disabled={disabled}
310320
className={classes.button}

packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleDrawer.test.tsx

Lines changed: 160 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
classifyIPs,
1414
deriveTypeFromValuesAndIPs,
1515
formValueToIPs,
16-
getInitialIPs,
16+
getInitialIPsOrPLs,
1717
IP_ERROR_MESSAGE,
1818
itemsToPortString,
1919
portStringToItems,
@@ -275,17 +275,25 @@ describe('ViewRuleSetDetailsDrawer', () => {
275275
describe('utilities', () => {
276276
describe('formValueToIPs', () => {
277277
it('returns a complete set of IPs given a string form value', () => {
278-
expect(formValueToIPs('all', [''].map(stringToExtendedIP))).toEqual(
278+
expect(formValueToIPs('all', [''].map(stringToExtendedIP), [])).toEqual(
279279
allIPs
280280
);
281-
expect(formValueToIPs('allIPv4', [''].map(stringToExtendedIP))).toEqual({
281+
expect(
282+
formValueToIPs('allIPv4', [''].map(stringToExtendedIP), [])
283+
).toEqual({
282284
ipv4: ['0.0.0.0/0'],
283285
});
284-
expect(formValueToIPs('allIPv6', [''].map(stringToExtendedIP))).toEqual({
286+
expect(
287+
formValueToIPs('allIPv6', [''].map(stringToExtendedIP), [])
288+
).toEqual({
285289
ipv6: ['::/0'],
286290
});
287291
expect(
288-
formValueToIPs('ip/netmask', ['1.1.1.1'].map(stringToExtendedIP))
292+
formValueToIPs(
293+
'ip/netmask/prefixlist',
294+
['1.1.1.1'].map(stringToExtendedIP),
295+
[]
296+
)
289297
).toEqual({
290298
ipv4: ['1.1.1.1'],
291299
});
@@ -304,22 +312,27 @@ describe('utilities', () => {
304312
});
305313

306314
describe('validateForm', () => {
315+
const baseOptions = {
316+
validatedIPs: [],
317+
validatedPLs: [],
318+
isFirewallRulesetsPrefixlistsFeatureEnabled: false,
319+
};
320+
307321
it('validates protocol', () => {
308-
expect(validateForm({})).toHaveProperty(
322+
expect(validateForm({}, baseOptions)).toHaveProperty(
309323
'protocol',
310324
'Protocol is required.'
311325
);
312326
});
313327
it('validates ports', () => {
314-
expect(validateForm({ ports: '80', protocol: 'ICMP' })).toHaveProperty(
315-
'ports',
316-
'Ports are not allowed for ICMP protocols.'
317-
);
318328
expect(
319-
validateForm({ ports: '443', protocol: 'IPENCAP' })
329+
validateForm({ ports: '80', protocol: 'ICMP' }, baseOptions)
330+
).toHaveProperty('ports', 'Ports are not allowed for ICMP protocols.');
331+
expect(
332+
validateForm({ ports: '443', protocol: 'IPENCAP' }, baseOptions)
320333
).toHaveProperty('ports', 'Ports are not allowed for IPENCAP protocols.');
321334
expect(
322-
validateForm({ ports: 'invalid-port', protocol: 'TCP' })
335+
validateForm({ ports: 'invalid-port', protocol: 'TCP' }, baseOptions)
323336
).toHaveProperty('ports');
324337
});
325338
it('validates custom ports', () => {
@@ -328,56 +341,77 @@ describe('utilities', () => {
328341
label: 'Firewalllabel',
329342
};
330343
// SUCCESS CASES
331-
expect(validateForm({ ports: '1234', protocol: 'TCP', ...rest })).toEqual(
332-
{}
333-
);
334344
expect(
335-
validateForm({ ports: '1,2,3,4,5', protocol: 'TCP', ...rest })
345+
validateForm({ ports: '1234', protocol: 'TCP', ...rest }, baseOptions)
336346
).toEqual({});
337347
expect(
338-
validateForm({ ports: '1, 2, 3, 4, 5', protocol: 'TCP', ...rest })
348+
validateForm(
349+
{ ports: '1,2,3,4,5', protocol: 'TCP', ...rest },
350+
baseOptions
351+
)
339352
).toEqual({});
340-
expect(validateForm({ ports: '1-20', protocol: 'TCP', ...rest })).toEqual(
341-
{}
342-
);
343353
expect(
344-
validateForm({
345-
ports: '1,2,3,4,5,6,7,8,9,10,11,12,13,14,15',
346-
protocol: 'TCP',
347-
...rest,
348-
})
354+
validateForm(
355+
{ ports: '1, 2, 3, 4, 5', protocol: 'TCP', ...rest },
356+
baseOptions
357+
)
358+
).toEqual({});
359+
expect(
360+
validateForm({ ports: '1-20', protocol: 'TCP', ...rest }, baseOptions)
361+
).toEqual({});
362+
expect(
363+
validateForm(
364+
{
365+
ports: '1,2,3,4,5,6,7,8,9,10,11,12,13,14,15',
366+
protocol: 'TCP',
367+
...rest,
368+
},
369+
baseOptions
370+
)
349371
).toEqual({});
350372
expect(
351-
validateForm({ ports: '1-2,3-4', protocol: 'TCP', ...rest })
373+
validateForm(
374+
{ ports: '1-2,3-4', protocol: 'TCP', ...rest },
375+
baseOptions
376+
)
352377
).toEqual({});
353378
expect(
354-
validateForm({ ports: '1,5-12', protocol: 'TCP', ...rest })
379+
validateForm({ ports: '1,5-12', protocol: 'TCP', ...rest }, baseOptions)
355380
).toEqual({});
356381
// FAILURE CASES
357382
expect(
358-
validateForm({ ports: '1,21-12', protocol: 'TCP', ...rest })
383+
validateForm(
384+
{ ports: '1,21-12', protocol: 'TCP', ...rest },
385+
baseOptions
386+
)
359387
).toHaveProperty(
360388
'ports',
361389
'Range must start with a smaller number and end with a larger number'
362390
);
363391
expect(
364-
validateForm({ ports: '1-21-45', protocol: 'TCP', ...rest })
392+
validateForm(
393+
{ ports: '1-21-45', protocol: 'TCP', ...rest },
394+
baseOptions
395+
)
365396
).toHaveProperty('ports', 'Ranges must have 2 values');
366397
expect(
367-
validateForm({ ports: 'abc', protocol: 'TCP', ...rest })
398+
validateForm({ ports: 'abc', protocol: 'TCP', ...rest }, baseOptions)
368399
).toHaveProperty('ports', 'Must be 1-65535');
369400
expect(
370-
validateForm({ ports: '1--20', protocol: 'TCP', ...rest })
401+
validateForm({ ports: '1--20', protocol: 'TCP', ...rest }, baseOptions)
371402
).toHaveProperty('ports', 'Must be 1-65535');
372403
expect(
373-
validateForm({ ports: '-20', protocol: 'TCP', ...rest })
404+
validateForm({ ports: '-20', protocol: 'TCP', ...rest }, baseOptions)
374405
).toHaveProperty('ports', 'Must be 1-65535');
375406
expect(
376-
validateForm({
377-
ports: '1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16',
378-
protocol: 'TCP',
379-
...rest,
380-
})
407+
validateForm(
408+
{
409+
ports: '1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16',
410+
protocol: 'TCP',
411+
...rest,
412+
},
413+
baseOptions
414+
)
381415
).toHaveProperty(
382416
'ports',
383417
'Number of ports or port range endpoints exceeded. Max allowed is 15'
@@ -432,11 +466,79 @@ describe('utilities', () => {
432466
ports: '80',
433467
protocol: 'TCP',
434468
};
435-
expect(validateForm({ label: value, ...rest })).toEqual(result);
469+
expect(validateForm({ label: value, ...rest }, baseOptions)).toEqual(
470+
result
471+
);
436472
});
437473
});
474+
475+
it('handles addresses field when isFirewallRulesetsPrefixlistsFeatureEnabled is true', () => {
476+
// Invalid cases
477+
expect(
478+
validateForm(
479+
{},
480+
{
481+
...baseOptions,
482+
isFirewallRulesetsPrefixlistsFeatureEnabled: true,
483+
}
484+
)
485+
).toHaveProperty('addresses', 'Sources is a required field.');
486+
487+
expect(
488+
validateForm(
489+
{ addresses: 'ip/netmask/prefixlist' },
490+
{
491+
...baseOptions,
492+
isFirewallRulesetsPrefixlistsFeatureEnabled: true,
493+
}
494+
)
495+
).toHaveProperty(
496+
'addresses',
497+
'Add an IP address in IP/mask format, or reference a Prefix List name.'
498+
);
499+
500+
// Valid cases
501+
expect(
502+
validateForm(
503+
{ addresses: 'ip/netmask/prefixlist' },
504+
{
505+
validatedIPs: [
506+
{ address: '192.268.0.0' },
507+
{ address: '192.268.0.1' },
508+
],
509+
validatedPLs: [
510+
{ address: 'pl:system:test', inIPv4Rule: true, inIPv6Rule: true },
511+
],
512+
isFirewallRulesetsPrefixlistsFeatureEnabled: true,
513+
}
514+
)
515+
).not.toHaveProperty('addresses');
516+
expect(
517+
validateForm(
518+
{ addresses: 'ip/netmask/prefixlist' },
519+
{
520+
validatedIPs: [{ address: '192.268.0.0' }],
521+
validatedPLs: [],
522+
isFirewallRulesetsPrefixlistsFeatureEnabled: true,
523+
}
524+
)
525+
).not.toHaveProperty('addresses');
526+
expect(
527+
validateForm(
528+
{ addresses: 'ip/netmask/prefixlist' },
529+
{
530+
validatedIPs: [],
531+
validatedPLs: [
532+
{ address: 'pl:system:test', inIPv4Rule: true, inIPv6Rule: true },
533+
],
534+
isFirewallRulesetsPrefixlistsFeatureEnabled: true,
535+
}
536+
)
537+
).not.toHaveProperty('addresses');
538+
});
539+
438540
it('handles required fields', () => {
439-
expect(validateForm({})).toEqual({
541+
expect(validateForm({}, baseOptions)).toEqual({
440542
addresses: 'Sources is a required field.',
441543
label: 'Label is required.',
442544
ports: 'Ports is a required field.',
@@ -445,11 +547,11 @@ describe('utilities', () => {
445547
});
446548
});
447549

448-
describe('getInitialIPs', () => {
550+
describe('getInitialIPsOrPLs', () => {
449551
const ruleToModify: ExtendedFirewallRule = {
450552
action: 'ACCEPT',
451553
addresses: {
452-
ipv4: ['1.2.3.4'],
554+
ipv4: ['1.2.3.4', 'pl:system:test'],
453555
ipv6: ['::0'],
454556
},
455557
originalIndex: 0,
@@ -458,10 +560,8 @@ describe('utilities', () => {
458560
status: 'NEW',
459561
};
460562
it('parses the IPs when no errors', () => {
461-
expect(getInitialIPs(ruleToModify)).toEqual([
462-
{ address: '1.2.3.4' },
463-
{ address: '::0' },
464-
]);
563+
const { ips: initalIPs } = getInitialIPsOrPLs(ruleToModify);
564+
expect(initalIPs).toEqual([{ address: '1.2.3.4' }, { address: '::0' }]);
465565
});
466566
it('parses the IPs with no errors', () => {
467567
const errors: FirewallRuleError[] = [
@@ -473,13 +573,17 @@ describe('utilities', () => {
473573
reason: 'Invalid IP',
474574
},
475575
];
476-
expect(getInitialIPs({ ...ruleToModify, errors })).toEqual([
576+
const { ips: initalIPs } = getInitialIPsOrPLs({
577+
...ruleToModify,
578+
errors,
579+
});
580+
expect(initalIPs).toEqual([
477581
{ address: '1.2.3.4', error: IP_ERROR_MESSAGE },
478582
{ address: '::0' },
479583
]);
480584
});
481585
it('offsets error indices correctly', () => {
482-
const result = getInitialIPs({
586+
const { ips: initialIPs } = getInitialIPsOrPLs({
483587
...ruleToModify,
484588
addresses: {
485589
ipv4: ['1.2.3.4'],
@@ -495,11 +599,17 @@ describe('utilities', () => {
495599
},
496600
],
497601
});
498-
expect(result).toEqual([
602+
expect(initialIPs).toEqual([
499603
{ address: '1.2.3.4' },
500604
{ address: 'INVALID_IP', error: IP_ERROR_MESSAGE },
501605
]);
502606
});
607+
it('parses the PLs when no errors', () => {
608+
const { pls: initalPLs } = getInitialIPsOrPLs(ruleToModify);
609+
expect(initalPLs).toEqual([
610+
{ address: 'pl:system:test', inIPv4Rule: true, inIPv6Rule: false },
611+
]);
612+
});
503613
});
504614

505615
describe('classifyIPs', () => {
@@ -528,12 +638,13 @@ describe('utilities', () => {
528638
};
529639

530640
it('correctly matches values to their representative type', () => {
531-
const result = deriveTypeFromValuesAndIPs(formValues, []);
641+
const result = deriveTypeFromValuesAndIPs(formValues, [], []);
532642
expect(result).toBe('https');
533643
});
534644
it('returns "custom" if there is no match', () => {
535645
const result = deriveTypeFromValuesAndIPs(
536646
{ ...formValues, ports: '22-23' },
647+
[],
537648
[]
538649
);
539650
expect(result).toBe('custom');

0 commit comments

Comments
 (0)