Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: [M3-7168] - Increment Subnet IPv4 value when recommending new subnet range #10010

Merged
merged 8 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10010-added-1703035008158.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Added
---

Subnet IPv4 range recommendation in VPC Create flow and Subnet Create Drawer ([#10010](https://github.com/linode/manager/pull/10010))
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ import { Link } from 'src/components/Link';
import { Notice } from 'src/components/Notice/Notice';
import { SubnetFieldState } from 'src/utilities/subnets';

import { VPC_CREATE_FORM_SUBNET_HELPER_TEXT } from '../../constants';
import { MultipleSubnetInput } from '../MultipleSubnetInput';
import {
StyledBodyTypography,
StyledHeaderTypography,
} from './VPCCreateForm.styles';

import { VPC_CREATE_FORM_SUBNET_HELPER_TEXT } from '../../constants';

interface Props {
disabled?: boolean;
isDrawer?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('MultipleSubnetInput', () => {
expect(props.onChange).toHaveBeenCalledWith([
...props.subnets,
{
ip: { availIPv4s: 256, ipv4: '10.0.4.0/24', ipv4Error: '' },
ip: { availIPv4s: 256, ipv4: '10.0.1.0/24', ipv4Error: '' },
label: '',
labelError: '',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Divider } from 'src/components/Divider';
import {
DEFAULT_SUBNET_IPV4_VALUE,
SubnetFieldState,
getRecommendedSubnetIPv4,
} from 'src/utilities/subnets';

import { SubnetNode } from './SubnetNode';
Expand All @@ -20,11 +21,20 @@ interface Props {
export const MultipleSubnetInput = (props: Props) => {
const { disabled, isDrawer, onChange, subnets } = props;

const [lastRecommendedIPv4, setLastRecommendedIPv4] = React.useState(
DEFAULT_SUBNET_IPV4_VALUE
);

const addSubnet = () => {
const recommendedIPv4 = getRecommendedSubnetIPv4(
lastRecommendedIPv4,
subnets.map((subnet) => subnet.ip.ipv4 ?? '')
);
setLastRecommendedIPv4(recommendedIPv4);
onChange([
...subnets,
{
ip: { availIPv4s: 256, ipv4: DEFAULT_SUBNET_IPV4_VALUE, ipv4Error: '' },
ip: { availIPv4s: 256, ipv4: recommendedIPv4, ipv4Error: '' },
label: '',
labelError: '',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,6 @@ describe('VPC create page', () => {
const { getAllByTestId } = renderWithTheme(<VPCCreate />);
const subnetIP = getAllByTestId('textfield-input');
expect(subnetIP[4]).toBeInTheDocument();
expect(subnetIP[4]).toHaveValue('10.0.4.0/24');
expect(subnetIP[4]).toHaveValue('10.0.0.0/24');
});
});
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { fireEvent } from '@testing-library/react';
import * as React from 'react';

import { renderWithTheme } from 'src/utilities/testHelpers';

import { SubnetCreateDrawer } from './SubnetCreateDrawer';
import { fireEvent } from '@testing-library/react';

const props = {
onClose: vi.fn(),
Expand All @@ -13,7 +13,7 @@ const props = {

describe('Create Subnet Drawer', () => {
it('should render title, label, ipv4 input, ipv4 availability, and action buttons', () => {
const { getByTestId, getAllByText, getByText } = renderWithTheme(
const { getAllByText, getByTestId, getByText } = renderWithTheme(
<SubnetCreateDrawer {...props} />
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
import { Drawer } from 'src/components/Drawer';
import { Notice } from 'src/components/Notice/Notice';
import { useGrants, useProfile } from 'src/queries/profile';
import { useCreateSubnetMutation } from 'src/queries/vpcs';
import { useCreateSubnetMutation, useVPCQuery } from 'src/queries/vpcs';
import { getErrorMap } from 'src/utilities/errorUtils';
import {
DEFAULT_SUBNET_IPV4_VALUE,
SubnetFieldState,
getRecommendedSubnetIPv4,
} from 'src/utilities/subnets';

import { SubnetNode } from '../VPCCreate/SubnetNode';
Expand All @@ -26,9 +27,15 @@ export const SubnetCreateDrawer = (props: Props) => {

const { data: profile } = useProfile();
const { data: grants } = useGrants();
const { data: vpc } = useVPCQuery(vpcId, open);

const userCannotAddSubnet = profile?.restricted && !grants?.global.add_vpcs;

const recommendedIPv4 = getRecommendedSubnetIPv4(
DEFAULT_SUBNET_IPV4_VALUE,
vpc?.subnets?.map((subnet) => subnet.ipv4 ?? '') ?? []
);

const [errorMap, setErrorMap] = React.useState<
Record<string, string | undefined>
>({});
Expand All @@ -41,31 +48,31 @@ export const SubnetCreateDrawer = (props: Props) => {

const onCreateSubnet = async () => {
try {
await createSubnet({ label: values.label, ipv4: values.ip.ipv4 });
await createSubnet({ ipv4: values.ip.ipv4, label: values.label });
onClose();
} catch (errors) {
const newErrors = getErrorMap(['label', 'ipv4'], errors);
setErrorMap(newErrors);
setValues({
label: values.label,
labelError: newErrors.label,
ip: {
...values.ip,
ipv4Error: newErrors.ipv4,
},
label: values.label,
labelError: newErrors.label,
});
}
};

const { dirty, handleSubmit, resetForm, setValues, values } = useFormik({
enableReinitialize: true,
initialValues: {
// @TODO VPC: add IPv6 when that is supported
label: '',
ip: {
ipv4: DEFAULT_SUBNET_IPV4_VALUE,
availIPv4s: 256,
ipv4: recommendedIPv4,
},
// @TODO VPC: add IPv6 when that is supported
label: '',
} as SubnetFieldState,
onSubmit: onCreateSubnet,
validateOnBlur: false,
Expand All @@ -79,7 +86,7 @@ export const SubnetCreateDrawer = (props: Props) => {
reset();
setErrorMap({});
}
}, [open]);
}, [open, reset, resetForm]);

return (
<Drawer onClose={onClose} open={open} title={'Create Subnet'}>
Expand All @@ -96,10 +103,10 @@ export const SubnetCreateDrawer = (props: Props) => {
)}
<form onSubmit={handleSubmit}>
<SubnetNode
disabled={userCannotAddSubnet}
onChange={(subnetState) => {
setValues(subnetState);
}}
disabled={userCannotAddSubnet}
subnet={values}
/>
<ActionsPanel
Expand All @@ -108,8 +115,8 @@ export const SubnetCreateDrawer = (props: Props) => {
disabled: !dirty || userCannotAddSubnet,
label: 'Create Subnet',
loading: isLoading,
type: 'submit',
onClick: onCreateSubnet,
type: 'submit',
}}
secondaryButtonProps={{ label: 'Cancel', onClick: onClose }}
/>
Expand Down
63 changes: 58 additions & 5 deletions packages/manager/src/utilities/subnets.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { calculateAvailableIPv4sRFC1918 } from './subnets';
import {
DEFAULT_SUBNET_IPV4_VALUE,
calculateAvailableIPv4sRFC1918,
getRecommendedSubnetIPv4,
} from './subnets';

describe('calculateAvailableIPv4s', () => {
it('should return a number if the input is a valid RFC1918 IPv4 with a mask', () => {
Expand Down Expand Up @@ -26,25 +30,25 @@ describe('calculateAvailableIPv4s', () => {
});

it('should return undefined for valid, non RFC1918 ips', () => {
//10.x ips
// 10.x ips
const badIP10 = calculateAvailableIPv4sRFC1918('10.0.0.0/7');
expect(badIP10).toBeUndefined();

//172.x ips
// 172.x ips
const badIP17215 = calculateAvailableIPv4sRFC1918('172.15.0.0/24');
expect(badIP17215).toBeUndefined();
const badIP17232 = calculateAvailableIPv4sRFC1918('172.32.0.0/24');
expect(badIP17232).toBeUndefined();
const badIP172Mask = calculateAvailableIPv4sRFC1918('172.16.0.0/11');
expect(badIP172Mask).toBeUndefined();

//192.x ips
// 192.x ips
const badIPNot192168 = calculateAvailableIPv4sRFC1918('192.15.0.0/24');
expect(badIPNot192168).toBeUndefined();
const badIP192mask = calculateAvailableIPv4sRFC1918('192.168.0.0/15');
expect(badIP192mask).toBeUndefined();

//non 10x, 172x, or 168x ips:
// non 10x, 172x, or 168x ips:
const nonRFC1 = calculateAvailableIPv4sRFC1918('145.24.8.0/24');
expect(nonRFC1).toBeUndefined();
const nonRFC2 = calculateAvailableIPv4sRFC1918('247.9.82.128/16');
Expand Down Expand Up @@ -73,3 +77,52 @@ describe('calculateAvailableIPv4s', () => {
expect(badMask3).toBe(undefined);
});
});

describe('getRecommendedSubnetIPv4', () => {
it('should return the default IPv4 address if the calculated recommended IPv4 address is not an RFC1918 ipv4', () => {
const recommendedIP1 = getRecommendedSubnetIPv4('10.0.255.0', []);
expect(recommendedIP1).toEqual(DEFAULT_SUBNET_IPV4_VALUE);

const recommendedIP2 = getRecommendedSubnetIPv4('bad ip', []);
expect(recommendedIP2).toEqual(DEFAULT_SUBNET_IPV4_VALUE);

const recommendedIP3 = getRecommendedSubnetIPv4('192.0.0.0/24', []);
expect(recommendedIP3).toEqual(DEFAULT_SUBNET_IPV4_VALUE);

const recommendedIP4 = getRecommendedSubnetIPv4('10.0.0.0/7', []);
expect(recommendedIP4).toEqual(DEFAULT_SUBNET_IPV4_VALUE);

const recommendedIP5 = getRecommendedSubnetIPv4('172.8.0.0/24', []);
expect(recommendedIP5).toEqual(DEFAULT_SUBNET_IPV4_VALUE);

const recommendedIP6 = getRecommendedSubnetIPv4('192.168.0.0/15', []);
expect(recommendedIP6).toEqual(DEFAULT_SUBNET_IPV4_VALUE);
});

it('should properly recommend an IPv4 by incrementing the third octet value by 1, if possible', () => {
const recommendedIP = getRecommendedSubnetIPv4('10.0.24.0/24', [
'10.0.0.0',
'10.0.1.0',
]);
expect(recommendedIP).toEqual('10.0.25.0/24');

const recommendedIP2 = getRecommendedSubnetIPv4('192.168.1.0/24', [
'10.0.0.0',
'192.168.0/24',
]);
expect(recommendedIP2).toEqual('192.168.2.0/24');
});

it('should recommend an IPv4 not already in the list of existing IPv4s', () => {
const recommendedIP = getRecommendedSubnetIPv4('10.0.5.0/24', [
'10.0.6.0/24',
'10.0.4.0/24',
]);
expect(recommendedIP).toEqual('10.0.7.0/24');
});

it('may recommend valid IPs that cover the same range', () => {
const recommendedIP = getRecommendedSubnetIPv4('172.16.0.0/16', []);
expect(recommendedIP).toEqual('172.16.1.0/16');
});
});
Loading