Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
2d8a834
Update Rules Edit/Add drawer to support PrefixLists
pmakode-akamai Nov 26, 2025
9a1cfd8
Update comment
pmakode-akamai Nov 26, 2025
d590fe0
Add some mocks for prefixlists
pmakode-akamai Nov 26, 2025
490712c
Update mock data order
pmakode-akamai Nov 26, 2025
755ef35
Reset pls state on address change
pmakode-akamai Nov 26, 2025
cdd979b
Feature-flagged add/edit prefixlists
pmakode-akamai Nov 26, 2025
4a9fb19
Some clean up
pmakode-akamai Nov 26, 2025
d562933
Some changes
pmakode-akamai Nov 26, 2025
0694313
More clean up
pmakode-akamai Nov 27, 2025
fc12b09
More clean up
pmakode-akamai Nov 27, 2025
8ba0548
Add code comments and some clean up
pmakode-akamai Nov 27, 2025
e8c5181
Add key and test ids to the pl rows
pmakode-akamai Nov 27, 2025
71e12ac
A small fix
pmakode-akamai Nov 27, 2025
2819213
Add unit tests
pmakode-akamai Nov 27, 2025
eaf77fc
More clean up
pmakode-akamai Nov 27, 2025
c1a640f
Rename component
pmakode-akamai Nov 27, 2025
dfd3ea2
Fix one test case
pmakode-akamai Nov 27, 2025
37dcd7e
Few fixes
pmakode-akamai Nov 27, 2025
ab80ec0
Fix type import
pmakode-akamai Nov 27, 2025
c6e6afd
Update mocks for PLs with more possible cases
pmakode-akamai Nov 28, 2025
994d227
Merge branch 'develop' into UIE-9515-update-rules-edit-and-add-drawer…
pmakode-akamai Nov 28, 2025
f110e65
Some more changes related to sorting
pmakode-akamai Nov 28, 2025
32eeee6
Merge branch 'develop' into UIE-9515-update-rules-edit-and-add-drawer…
pmakode-akamai Dec 1, 2025
c01df71
Update mocks
pmakode-akamai Dec 1, 2025
71b6f4b
Merge branch 'develop' into UIE-9515-update-rules-edit-and-add-drawer…
pmakode-akamai Dec 1, 2025
d1ce6e3
Resolve merge conflicts
pmakode-akamai Dec 2, 2025
b7b3194
Added changeset: Update Firewall Rules Edit & Add Drawer to Support P…
pmakode-akamai Dec 2, 2025
e770826
Add/update comments
pmakode-akamai Dec 2, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Update Firewall Rules Edit & Add Drawer to Support Prefix List Selection ([#13138](https://github.com/linode/manager/pull/13138))
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const useStyles = makeStyles()((theme: Theme) => ({
justifyContent: 'flex-start',
},
paddingLeft: 0,
paddingTop: theme.spacing(1.5),
paddingTop: theme.spacingFunction(12),
},
button: {
'& > span': {
Expand Down Expand Up @@ -70,6 +70,12 @@ export interface MultipeIPInputProps {
*/
buttonText?: string;

/**
* Whether the first input field can be removed.
* @default false
*/
canRemoveFirstInput?: boolean;

/**
* Custom CSS class for additional styling.
*/
Expand Down Expand Up @@ -155,6 +161,7 @@ export const MultipleIPInput = React.memo((props: MultipeIPInputProps) => {
const {
adjustSpacingForVPCDualStack,
buttonText,
canRemoveFirstInput,
className,
disabled,
error,
Expand Down Expand Up @@ -244,8 +251,8 @@ export const MultipleIPInput = React.memo((props: MultipeIPInputProps) => {
<TooltipIcon
status="info"
sxTooltipIcon={{
marginLeft: '-4px',
marginTop: '-15px',
marginTop: '-8px',
padding: '4px',
}}
text={tooltip}
tooltipPosition="right"
Expand Down Expand Up @@ -304,7 +311,10 @@ export const MultipleIPInput = React.memo((props: MultipeIPInputProps) => {
* used in DBaaS or for Linode VPC interfaces
*/}
<Grid size={1}>
{(idx > 0 || forDatabaseAccessControls || forVPCIPRanges) && (
{(idx > 0 ||
forDatabaseAccessControls ||
forVPCIPRanges ||
canRemoveFirstInput) && (
<IconButton
aria-disabled={disabled}
className={classes.button}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
classifyIPs,
deriveTypeFromValuesAndIPs,
formValueToIPs,
getInitialIPs,
getInitialIPsOrPLs,
IP_ERROR_MESSAGE,
itemsToPortString,
portStringToItems,
Expand Down Expand Up @@ -275,17 +275,25 @@
describe('utilities', () => {
describe('formValueToIPs', () => {
it('returns a complete set of IPs given a string form value', () => {
expect(formValueToIPs('all', [''].map(stringToExtendedIP))).toEqual(
expect(formValueToIPs('all', [''].map(stringToExtendedIP), [])).toEqual(
allIPs
);
expect(formValueToIPs('allIPv4', [''].map(stringToExtendedIP))).toEqual({
expect(
formValueToIPs('allIPv4', [''].map(stringToExtendedIP), [])
).toEqual({
ipv4: ['0.0.0.0/0'],
});
expect(formValueToIPs('allIPv6', [''].map(stringToExtendedIP))).toEqual({
expect(
formValueToIPs('allIPv6', [''].map(stringToExtendedIP), [])
).toEqual({
ipv6: ['::/0'],
});
expect(
formValueToIPs('ip/netmask', ['1.1.1.1'].map(stringToExtendedIP))
formValueToIPs(
'ip/netmask/prefixlist',

Check warning on line 293 in packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleDrawer.test.tsx

View workflow job for this annotation

GitHub Actions / ESLint Review (manager)

[eslint] reported by reviewdog 🐢 Define a constant instead of duplicating this literal 5 times. Raw Output: {"ruleId":"sonarjs/no-duplicate-string","severity":1,"message":"Define a constant instead of duplicating this literal 5 times.","line":293,"column":11,"nodeType":"Literal","endLine":293,"endColumn":34}
['1.1.1.1'].map(stringToExtendedIP),
[]
)
).toEqual({
ipv4: ['1.1.1.1'],
});
Expand All @@ -304,22 +312,27 @@
});

describe('validateForm', () => {
const baseOptions = {
validatedIPs: [],
validatedPLs: [],
isFirewallRulesetsPrefixlistsFeatureEnabled: false,
};

it('validates protocol', () => {
expect(validateForm({})).toHaveProperty(
expect(validateForm({}, baseOptions)).toHaveProperty(
'protocol',
'Protocol is required.'
);
});
it('validates ports', () => {
expect(validateForm({ ports: '80', protocol: 'ICMP' })).toHaveProperty(
'ports',
'Ports are not allowed for ICMP protocols.'
);
expect(
validateForm({ ports: '443', protocol: 'IPENCAP' })
validateForm({ ports: '80', protocol: 'ICMP' }, baseOptions)
).toHaveProperty('ports', 'Ports are not allowed for ICMP protocols.');
expect(
validateForm({ ports: '443', protocol: 'IPENCAP' }, baseOptions)
).toHaveProperty('ports', 'Ports are not allowed for IPENCAP protocols.');
expect(
validateForm({ ports: 'invalid-port', protocol: 'TCP' })
validateForm({ ports: 'invalid-port', protocol: 'TCP' }, baseOptions)
).toHaveProperty('ports');
});
it('validates custom ports', () => {
Expand All @@ -328,56 +341,77 @@
label: 'Firewalllabel',
};
// SUCCESS CASES
expect(validateForm({ ports: '1234', protocol: 'TCP', ...rest })).toEqual(
{}
);
expect(
validateForm({ ports: '1,2,3,4,5', protocol: 'TCP', ...rest })
validateForm({ ports: '1234', protocol: 'TCP', ...rest }, baseOptions)
).toEqual({});
expect(
validateForm({ ports: '1, 2, 3, 4, 5', protocol: 'TCP', ...rest })
validateForm(
{ ports: '1,2,3,4,5', protocol: 'TCP', ...rest },
baseOptions
)
).toEqual({});
expect(validateForm({ ports: '1-20', protocol: 'TCP', ...rest })).toEqual(
{}
);
expect(
validateForm({
ports: '1,2,3,4,5,6,7,8,9,10,11,12,13,14,15',
protocol: 'TCP',
...rest,
})
validateForm(
{ ports: '1, 2, 3, 4, 5', protocol: 'TCP', ...rest },
baseOptions
)
).toEqual({});
expect(
validateForm({ ports: '1-20', protocol: 'TCP', ...rest }, baseOptions)
).toEqual({});
expect(
validateForm(
{
ports: '1,2,3,4,5,6,7,8,9,10,11,12,13,14,15',
protocol: 'TCP',
...rest,
},
baseOptions
)
).toEqual({});
expect(
validateForm({ ports: '1-2,3-4', protocol: 'TCP', ...rest })
validateForm(
{ ports: '1-2,3-4', protocol: 'TCP', ...rest },
baseOptions
)
).toEqual({});
expect(
validateForm({ ports: '1,5-12', protocol: 'TCP', ...rest })
validateForm({ ports: '1,5-12', protocol: 'TCP', ...rest }, baseOptions)
).toEqual({});
// FAILURE CASES
expect(
validateForm({ ports: '1,21-12', protocol: 'TCP', ...rest })
validateForm(
{ ports: '1,21-12', protocol: 'TCP', ...rest },
baseOptions
)
).toHaveProperty(
'ports',
'Range must start with a smaller number and end with a larger number'
);
expect(
validateForm({ ports: '1-21-45', protocol: 'TCP', ...rest })
validateForm(
{ ports: '1-21-45', protocol: 'TCP', ...rest },
baseOptions
)
).toHaveProperty('ports', 'Ranges must have 2 values');
expect(
validateForm({ ports: 'abc', protocol: 'TCP', ...rest })
validateForm({ ports: 'abc', protocol: 'TCP', ...rest }, baseOptions)
).toHaveProperty('ports', 'Must be 1-65535');
expect(
validateForm({ ports: '1--20', protocol: 'TCP', ...rest })
validateForm({ ports: '1--20', protocol: 'TCP', ...rest }, baseOptions)
).toHaveProperty('ports', 'Must be 1-65535');
expect(
validateForm({ ports: '-20', protocol: 'TCP', ...rest })
validateForm({ ports: '-20', protocol: 'TCP', ...rest }, baseOptions)
).toHaveProperty('ports', 'Must be 1-65535');
expect(
validateForm({
ports: '1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16',
protocol: 'TCP',
...rest,
})
validateForm(
{
ports: '1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16',
protocol: 'TCP',
...rest,
},
baseOptions
)
).toHaveProperty(
'ports',
'Number of ports or port range endpoints exceeded. Max allowed is 15'
Expand Down Expand Up @@ -432,11 +466,79 @@
ports: '80',
protocol: 'TCP',
};
expect(validateForm({ label: value, ...rest })).toEqual(result);
expect(validateForm({ label: value, ...rest }, baseOptions)).toEqual(
result
);
});
});

it('handles addresses field when isFirewallRulesetsPrefixlistsFeatureEnabled is true', () => {
// Invalid cases
expect(
validateForm(
{},
{
...baseOptions,
isFirewallRulesetsPrefixlistsFeatureEnabled: true,
}
)
).toHaveProperty('addresses', 'Sources is a required field.');

expect(
validateForm(
{ addresses: 'ip/netmask/prefixlist' },
{
...baseOptions,
isFirewallRulesetsPrefixlistsFeatureEnabled: true,
}
)
).toHaveProperty(
'addresses',
'Add an IP address in IP/mask format, or reference a Prefix List name.'
);

// Valid cases
expect(
validateForm(
{ addresses: 'ip/netmask/prefixlist' },
{
validatedIPs: [
{ address: '192.268.0.0' },
{ address: '192.268.0.1' },
],
validatedPLs: [
{ address: 'pl:system:test', inIPv4Rule: true, inIPv6Rule: true },

Check warning on line 510 in packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleDrawer.test.tsx

View workflow job for this annotation

GitHub Actions / ESLint Review (manager)

[eslint] reported by reviewdog 🐢 Define a constant instead of duplicating this literal 4 times. Raw Output: {"ruleId":"sonarjs/no-duplicate-string","severity":1,"message":"Define a constant instead of duplicating this literal 4 times.","line":510,"column":26,"nodeType":"Literal","endLine":510,"endColumn":42}
],
isFirewallRulesetsPrefixlistsFeatureEnabled: true,
}
)
).not.toHaveProperty('addresses');
expect(
validateForm(
{ addresses: 'ip/netmask/prefixlist' },
{
validatedIPs: [{ address: '192.268.0.0' }],
validatedPLs: [],
isFirewallRulesetsPrefixlistsFeatureEnabled: true,
}
)
).not.toHaveProperty('addresses');
expect(
validateForm(
{ addresses: 'ip/netmask/prefixlist' },
{
validatedIPs: [],
validatedPLs: [
{ address: 'pl:system:test', inIPv4Rule: true, inIPv6Rule: true },
],
isFirewallRulesetsPrefixlistsFeatureEnabled: true,
}
)
).not.toHaveProperty('addresses');
});

it('handles required fields', () => {
expect(validateForm({})).toEqual({
expect(validateForm({}, baseOptions)).toEqual({
addresses: 'Sources is a required field.',
label: 'Label is required.',
ports: 'Ports is a required field.',
Expand All @@ -445,11 +547,11 @@
});
});

describe('getInitialIPs', () => {
describe('getInitialIPsOrPLs', () => {
const ruleToModify: ExtendedFirewallRule = {
action: 'ACCEPT',
addresses: {
ipv4: ['1.2.3.4'],
ipv4: ['1.2.3.4', 'pl:system:test'],
ipv6: ['::0'],
},
originalIndex: 0,
Expand All @@ -458,10 +560,8 @@
status: 'NEW',
};
it('parses the IPs when no errors', () => {
expect(getInitialIPs(ruleToModify)).toEqual([
{ address: '1.2.3.4' },
{ address: '::0' },
]);
const { ips: initalIPs } = getInitialIPsOrPLs(ruleToModify);
expect(initalIPs).toEqual([{ address: '1.2.3.4' }, { address: '::0' }]);
});
it('parses the IPs with no errors', () => {
const errors: FirewallRuleError[] = [
Expand All @@ -473,13 +573,17 @@
reason: 'Invalid IP',
},
];
expect(getInitialIPs({ ...ruleToModify, errors })).toEqual([
const { ips: initalIPs } = getInitialIPsOrPLs({
...ruleToModify,
errors,
});
expect(initalIPs).toEqual([
{ address: '1.2.3.4', error: IP_ERROR_MESSAGE },
{ address: '::0' },
]);
});
it('offsets error indices correctly', () => {
const result = getInitialIPs({
const { ips: initialIPs } = getInitialIPsOrPLs({
...ruleToModify,
addresses: {
ipv4: ['1.2.3.4'],
Expand All @@ -495,11 +599,17 @@
},
],
});
expect(result).toEqual([
expect(initialIPs).toEqual([
{ address: '1.2.3.4' },
{ address: 'INVALID_IP', error: IP_ERROR_MESSAGE },
]);
});
it('parses the PLs when no errors', () => {
const { pls: initalPLs } = getInitialIPsOrPLs(ruleToModify);
expect(initalPLs).toEqual([
{ address: 'pl:system:test', inIPv4Rule: true, inIPv6Rule: false },
]);
});
});

describe('classifyIPs', () => {
Expand Down Expand Up @@ -528,12 +638,13 @@
};

it('correctly matches values to their representative type', () => {
const result = deriveTypeFromValuesAndIPs(formValues, []);
const result = deriveTypeFromValuesAndIPs(formValues, [], []);
expect(result).toBe('https');
});
it('returns "custom" if there is no match', () => {
const result = deriveTypeFromValuesAndIPs(
{ ...formValues, ports: '22-23' },
[],
[]
);
expect(result).toBe('custom');
Expand Down
Loading
Loading