From ba45d604949b77dfcd33e1048a5692a336eeeefe Mon Sep 17 00:00:00 2001 From: Connie Liu <139280159+coliu-akamai@users.noreply.github.com> Date: Thu, 18 Jan 2024 17:57:02 -0500 Subject: [PATCH] feat: [M3-7168] - Increment Subnet IPv4 value when recommending new subnet range (#10010) Co-authored-by: Banks Nussman --- .../pr-10010-added-1703035008158.md | 5 + .../FormComponents/SubnetContent.tsx | 3 +- .../VPCCreate/MultipleSubnetInput.test.tsx | 2 +- .../VPCs/VPCCreate/MultipleSubnetInput.tsx | 12 ++- .../VPCs/VPCCreate/VPCCreate.test.tsx | 2 +- .../VPCs/VPCDetail/SubnetCreateDrawer.tsx | 27 ++++-- .../manager/src/utilities/subnets.test.ts | 63 +++++++++++- packages/manager/src/utilities/subnets.ts | 97 +++++++++++++++---- 8 files changed, 173 insertions(+), 38 deletions(-) create mode 100644 packages/manager/.changeset/pr-10010-added-1703035008158.md diff --git a/packages/manager/.changeset/pr-10010-added-1703035008158.md b/packages/manager/.changeset/pr-10010-added-1703035008158.md new file mode 100644 index 00000000000..e5617d007d3 --- /dev/null +++ b/packages/manager/.changeset/pr-10010-added-1703035008158.md @@ -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)) diff --git a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx index 43cd8190cb2..fac5527d590 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx @@ -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; diff --git a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.test.tsx b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.test.tsx index 1c5b842b1ad..0babd9bf0f7 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.test.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.test.tsx @@ -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: '', }, diff --git a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.tsx b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.tsx index 874821a4914..23c3f1b42eb 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.tsx @@ -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'; @@ -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: '', }, diff --git a/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.test.tsx b/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.test.tsx index c5f5d019019..0617029c98d 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.test.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.test.tsx @@ -104,6 +104,6 @@ describe('VPC create page', () => { const { getAllByTestId } = renderWithTheme(); 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'); }); }); diff --git a/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx b/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx index 1e39fee014f..d7e613b0157 100644 --- a/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx +++ b/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx @@ -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'; @@ -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 >({}); @@ -41,18 +48,18 @@ 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, }); } }; @@ -60,12 +67,12 @@ export const SubnetCreateDrawer = (props: Props) => { 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, @@ -79,7 +86,7 @@ export const SubnetCreateDrawer = (props: Props) => { reset(); setErrorMap({}); } - }, [open]); + }, [open, reset, resetForm]); return ( @@ -96,10 +103,10 @@ export const SubnetCreateDrawer = (props: Props) => { )}
{ setValues(subnetState); }} + disabled={userCannotAddSubnet} subnet={values} /> { disabled: !dirty || userCannotAddSubnet, label: 'Create Subnet', loading: isLoading, - type: 'submit', onClick: onCreateSubnet, + type: 'submit', }} secondaryButtonProps={{ label: 'Cancel', onClick: onClose }} /> diff --git a/packages/manager/src/utilities/subnets.test.ts b/packages/manager/src/utilities/subnets.test.ts index c1cf9caf154..03a4a6de8ee 100644 --- a/packages/manager/src/utilities/subnets.test.ts +++ b/packages/manager/src/utilities/subnets.test.ts @@ -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', () => { @@ -26,11 +30,11 @@ 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'); @@ -38,13 +42,13 @@ describe('calculateAvailableIPv4s', () => { 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'); @@ -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'); + }); +}); diff --git a/packages/manager/src/utilities/subnets.ts b/packages/manager/src/utilities/subnets.ts index 1579ef09642..44ad6d4449a 100644 --- a/packages/manager/src/utilities/subnets.ts +++ b/packages/manager/src/utilities/subnets.ts @@ -1,6 +1,6 @@ import { determineIPType } from '@linode/validation'; -export const DEFAULT_SUBNET_IPV4_VALUE = '10.0.4.0/24'; +export const DEFAULT_SUBNET_IPV4_VALUE = '10.0.0.0/24'; export const RESERVED_IP_NUMBER = 4; export const SUBNET_LINODE_CSV_HEADERS = [ @@ -10,7 +10,7 @@ export const SUBNET_LINODE_CSV_HEADERS = [ ]; // @TODO VPC: added ipv6 related fields here, but they will not be used until VPCs support ipv6 -interface SubnetIPState { +export interface SubnetIPState { availIPv4s?: number; ipv4?: string; ipv4Error?: string; @@ -69,36 +69,97 @@ export const SubnetMaskToAvailIPv4s = { 32: 1, }; -export const calculateAvailableIPv4sRFC1918 = ( - address: string -): number | undefined => { +/** + * Determines if the given IPv4 is an RFC1918 IP address. + */ +const isValidRFC1918IPv4 = (address: string) => { const [ip, mask] = address.split('/'); const ipType = determineIPType(address); if (ipType !== 'ipv4' || mask === '' || mask === undefined) { - return undefined; + return false; } const [firstOctet, secondOctet] = ip.split('.'); const parsedMask = parseInt(mask, 10); const parsedSecondOctet = parseInt(secondOctet, 10); - // if the IP is not in the RFC1918 ranges, hold off on displaying number of available IPs. The ranges are: // 10.x.x.x (10/8 prefix) // 172.16.x.x-172.31.x.x (172/12 prefix) // 192.168.x.x (192.168/16 prefix) - if ( - (firstOctet !== '10' && firstOctet !== '172' && firstOctet !== '192') || - // Check for invalid 10.x IPs - (firstOctet === '10' && parsedMask < 8) || - // check for invalid 172.x IPs + return ( + // check for valid 10.x IPs + (firstOctet === '10' && parsedMask >= 8) || + // check for valid 172.x IPs (firstOctet === '172' && - (parsedSecondOctet < 16 || parsedSecondOctet > 31 || parsedMask < 12)) || - // check for invalid 192.x IPs - (firstOctet === '192' && - (secondOctet !== '168' || (secondOctet === '168' && parsedMask < 16))) + parsedSecondOctet >= 16 && + parsedSecondOctet <= 31 && + parsedMask >= 12) || + // check for valid 192.x IPs + (firstOctet === '192' && secondOctet === '168' && parsedMask >= 16) + ); +}; + +export const calculateAvailableIPv4sRFC1918 = ( + address: string +): number | undefined => { + const [, mask] = address.split('/'); + + // if the IP is not in the RFC1918 ranges, hold off on displaying number of available IPs + return isValidRFC1918IPv4(address) ? SubnetMaskToAvailIPv4s[mask] : undefined; +}; + +/** + * Calculates the next subnet IPv4 address to recommend when creating a subnet, based off of the last recommended ipv4 and already existing IPv4s, + * by incrementing the third octet by one. + * + * @param lastRecommendedIPv4 the current IPv4 address to base our recommended IPv4 address off of + * @param otherIPv4s the other IPv4s to check against + * @returns the next recommended subnet IPv4 address to use + * + * Assumption: if @param lastRecommendedIPv4 is a valid RFC1918 IPv4 and in x.x.x.x/x format, then the output is a valid RFC1918 IPv4 in x.x.x.x/x + * format and not already in @param otherIPv4s (excluding the default IPv4 case -- see comments below). HOWEVER, a recommended IP may still cover + * the same range as an existing IPv4 (ex 172.16.0.0/16 and 172.16.1.0/16 cover parts of the same range) and therefore not be accepted by the backend. + */ +export const getRecommendedSubnetIPv4 = ( + lastRecommendedIPv4: string, + otherIPv4s: string[] +): string => { + const [ + firstOctet, + secondOctet, + thirdOctet, + fourthOctet, + ] = lastRecommendedIPv4.split('.'); + const parsedThirdOctet = parseInt(thirdOctet, 10); + let ipv4ToReturn = ''; + + /** + * Return DEFAULT_SUBNET_IPV4_VALUE (10.0.0.0/24) if parsedThirdOctet + 1 would result in a nonsense ipv4 (ex. 10.0.256.0/24 is not an IPv4) + * Realistically this case will rarely be reached and acts mainly as a safety check: a) when creating a VPC, the first recommended address is + * always 10.0.0.0/24, and b) most people will be allowed a max of 10 subnets in their VPC, so there *should be* plenty of subnets to recommend + */ + if ( + !isValidRFC1918IPv4(lastRecommendedIPv4) || + isNaN(parsedThirdOctet) || + parsedThirdOctet + 1 > 255 + ) { + return DEFAULT_SUBNET_IPV4_VALUE; + } else { + ipv4ToReturn = `${firstOctet}.${secondOctet}.${ + parsedThirdOctet + 1 + }.${fourthOctet}`; + } + + // if the IPv4 we've recommended already exists, we recommend a new IP + if ( + otherIPv4s.some((ip) => { + const [_ip] = ip.split('/'); + const [_ipv4ToReturn] = ipv4ToReturn.split('/'); + return _ip === _ipv4ToReturn; + }) ) { - return undefined; + return getRecommendedSubnetIPv4(ipv4ToReturn, otherIPv4s); } - return SubnetMaskToAvailIPv4s[mask]; + return ipv4ToReturn; };