From 2505c1503ef792cd8f887122fa1185ee28b12ca8 Mon Sep 17 00:00:00 2001 From: coliu-akamai Date: Tue, 19 Dec 2023 14:20:54 -0500 Subject: [PATCH 1/8] started working on logic --- .../VPCCreate/MultipleSubnetInput.test.tsx | 2 +- .../VPCs/VPCCreate/MultipleSubnetInput.tsx | 12 ++++- .../VPCs/VPCCreate/VPCCreate.test.tsx | 2 +- packages/manager/src/utilities/subnets.ts | 50 ++++++++++++++++++- 4 files changed, 61 insertions(+), 5 deletions(-) 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..f93176cff6d 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 + ); + 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/utilities/subnets.ts b/packages/manager/src/utilities/subnets.ts index 1579ef09642..130f0228c3e 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; @@ -102,3 +102,49 @@ export const calculateAvailableIPv4sRFC1918 = ( return SubnetMaskToAvailIPv4s[mask]; }; + +/** + * Calculates the next subnet IPv4 address to recommend when creating a subnet, based off of the last recommended ipv4 and the rest of the other subnets. + * @param lastRecommendedIPv4 the current IPv4 address to base our recommended IPv4 address off of + * @param allSubnets the subnets to check the IPs of + * @returns the next recommended subnet IPv4 address to use + * + * Assumption: if the inputted ipv4 is valid and in x.x.x.x/x format, then the outputted ipv4 valid and in x.x.x.x/x format + */ +export function getRecommendedSubnetIPv4< + T extends { ip?: SubnetIPState; ipv4?: string } +>(lastRecommendedIPv4: string, allSubnets: T[]): 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 most people will be allowed a max of 10 subnets in their VPC and + * + * b) when creating a new subnet, we will suffer... + */ + if (isNaN(parsedThirdOctet) || parsedThirdOctet + 1 > 255) { + return DEFAULT_SUBNET_IPV4_VALUE; + } else { + ipv4ToReturn = `${firstOctet}.${secondOctet}.${ + parsedThirdOctet + 1 + }.${fourthOctet}`; + } + + if ( + allSubnets.some( + (subnet) => subnet.ip === ipv4ToReturn || subnet.ip?.ipv4 === ipv4ToReturn + ) + ) { + return getRecommendedSubnetIPv4(ipv4ToReturn, allSubnets); + } + + return ipv4ToReturn; +} From 4ba5115a3f9f30bd7480e0259d6b76af4a3fe4de Mon Sep 17 00:00:00 2001 From: coliu-akamai Date: Tue, 19 Dec 2023 15:44:12 -0500 Subject: [PATCH 2/8] subnet create drawer updates --- .../VPCs/VPCCreate/MultipleSubnetInput.tsx | 2 +- .../VPCs/VPCDetail/SubnetCreateDrawer.tsx | 30 ++++++++++++------- packages/manager/src/queries/vpcs.ts | 14 +++++++++ packages/manager/src/utilities/subnets.ts | 28 ++++++++--------- 4 files changed, 47 insertions(+), 27 deletions(-) diff --git a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.tsx b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.tsx index f93176cff6d..23c3f1b42eb 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.tsx @@ -28,7 +28,7 @@ export const MultipleSubnetInput = (props: Props) => { const addSubnet = () => { const recommendedIPv4 = getRecommendedSubnetIPv4( lastRecommendedIPv4, - subnets + subnets.map((subnet) => subnet.ip.ipv4 ?? '') ); setLastRecommendedIPv4(recommendedIPv4); onChange([ diff --git a/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx b/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx index 1e39fee014f..38907bd9f0e 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 { useAllSubnetsQuery, useCreateSubnetMutation } 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,18 @@ export const SubnetCreateDrawer = (props: Props) => { const { data: profile } = useProfile(); const { data: grants } = useGrants(); + const { data: allSubnets } = useAllSubnetsQuery(vpcId); const userCannotAddSubnet = profile?.restricted && !grants?.global.add_vpcs; + const recommendedIPv4 = + allSubnets && allSubnets.length > 0 + ? getRecommendedSubnetIPv4( + allSubnets[0].ipv4 ?? '', + allSubnets.map((subnet) => subnet.ipv4 ?? '') + ) + : DEFAULT_SUBNET_IPV4_VALUE; + const [errorMap, setErrorMap] = React.useState< Record >({}); @@ -41,18 +51,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 +70,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 +89,7 @@ export const SubnetCreateDrawer = (props: Props) => { reset(); setErrorMap({}); } - }, [open]); + }, [open, reset, resetForm]); return ( @@ -96,10 +106,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/queries/vpcs.ts b/packages/manager/src/queries/vpcs.ts index 10175e32a41..3693b9bd9a1 100644 --- a/packages/manager/src/queries/vpcs.ts +++ b/packages/manager/src/queries/vpcs.ts @@ -24,6 +24,8 @@ import { } from '@linode/api-v4/lib/types'; import { useMutation, useQuery, useQueryClient } from 'react-query'; +import { getAll } from 'src/utilities/getAll'; + export const vpcQueryKey = 'vpcs'; export const subnetQueryKey = 'subnets'; @@ -96,6 +98,18 @@ export const useSubnetsQuery = ( ); }; +export const useAllSubnetsQuery = (vpcID: number, enabled: boolean = true) => + useQuery( + [vpcQueryKey, 'vpc', vpcID, subnetQueryKey, 'unpaginated'], + () => getAllSubnetsRequest(vpcID), + { enabled, keepPreviousData: true } + ); + +const getAllSubnetsRequest = (vpcID: number) => + getAll((params, filters) => + getSubnets(vpcID, params, filters) + )().then((data) => data.data); + export const useSubnetQuery = (vpcID: number, subnetID: number) => { return useQuery( [vpcQueryKey, 'vpc', vpcID, subnetQueryKey, 'subnet', subnetID], diff --git a/packages/manager/src/utilities/subnets.ts b/packages/manager/src/utilities/subnets.ts index 130f0228c3e..d53ff5a5774 100644 --- a/packages/manager/src/utilities/subnets.ts +++ b/packages/manager/src/utilities/subnets.ts @@ -104,16 +104,18 @@ export const calculateAvailableIPv4sRFC1918 = ( }; /** - * Calculates the next subnet IPv4 address to recommend when creating a subnet, based off of the last recommended ipv4 and the rest of the other subnets. + * Calculates the next subnet IPv4 address to recommend when creating a subnet, based off of the last recommended ipv4 and already existing IPv4s. * @param lastRecommendedIPv4 the current IPv4 address to base our recommended IPv4 address off of - * @param allSubnets the subnets to check the IPs of + * @param otherIPv4s the other IPv4s to check against * @returns the next recommended subnet IPv4 address to use * - * Assumption: if the inputted ipv4 is valid and in x.x.x.x/x format, then the outputted ipv4 valid and in x.x.x.x/x format + * Assumption: if the inputted ipv4 is valid and in x.x.x.x/x format, then the outputted ipv4 valid and in x.x.x.x/x format not already in + * otherIPv4s (excluding the default case -- see comments below) */ -export function getRecommendedSubnetIPv4< - T extends { ip?: SubnetIPState; ipv4?: string } ->(lastRecommendedIPv4: string, allSubnets: T[]): string { +export const getRecommendedSubnetIPv4 = ( + lastRecommendedIPv4: string, + otherIPv4s: string[] +): string => { const [ firstOctet, secondOctet, @@ -126,9 +128,7 @@ export function getRecommendedSubnetIPv4< /** * 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 most people will be allowed a max of 10 subnets in their VPC and - * - * b) when creating a new subnet, we will suffer... + * always 10.0.0.0/24, and b) most people will be allowed a max of 10 subnets in their VPC, so there are plenty of subnets to recommend */ if (isNaN(parsedThirdOctet) || parsedThirdOctet + 1 > 255) { return DEFAULT_SUBNET_IPV4_VALUE; @@ -138,13 +138,9 @@ export function getRecommendedSubnetIPv4< }.${fourthOctet}`; } - if ( - allSubnets.some( - (subnet) => subnet.ip === ipv4ToReturn || subnet.ip?.ipv4 === ipv4ToReturn - ) - ) { - return getRecommendedSubnetIPv4(ipv4ToReturn, allSubnets); + if (otherIPv4s.some((ip) => ip === ipv4ToReturn)) { + return getRecommendedSubnetIPv4(ipv4ToReturn, otherIPv4s); } return ipv4ToReturn; -} +}; From 207c2d4d1efe99d474d0b324877917bfe414af59 Mon Sep 17 00:00:00 2001 From: coliu-akamai Date: Tue, 19 Dec 2023 19:49:16 -0500 Subject: [PATCH 3/8] add tests for vpc stuff --- .../FormComponents/SubnetContent.tsx | 3 +- .../VPCDetail/SubnetCreateDrawer.test.tsx | 4 +- .../VPCs/VPCDetail/SubnetCreateDrawer.tsx | 4 +- .../manager/src/utilities/subnets.test.ts | 63 +++++++++++++++++-- packages/manager/src/utilities/subnets.ts | 60 +++++++++++------- 5 files changed, 100 insertions(+), 34 deletions(-) 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/VPCDetail/SubnetCreateDrawer.test.tsx b/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.test.tsx index f614c8ae53c..c6881cb7028 100644 --- a/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.test.tsx +++ b/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.test.tsx @@ -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(), @@ -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( ); diff --git a/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx b/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx index 38907bd9f0e..77587af55ac 100644 --- a/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx +++ b/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx @@ -34,7 +34,9 @@ export const SubnetCreateDrawer = (props: Props) => { const recommendedIPv4 = allSubnets && allSubnets.length > 0 ? getRecommendedSubnetIPv4( - allSubnets[0].ipv4 ?? '', + // for simplicity purposes, we take the IPv4 of the last subnet in allSubnets, as + // getRecommendedSubnetIPv4 will iterate through potential IPv4s to recommend anyway + allSubnets[allSubnets.length - 1].ipv4 ?? '', allSubnets.map((subnet) => subnet.ipv4 ?? '') ) : DEFAULT_SUBNET_IPV4_VALUE; diff --git a/packages/manager/src/utilities/subnets.test.ts b/packages/manager/src/utilities/subnets.test.ts index c1cf9caf154..d20cc17d767 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('recommends IPs with a /24 mask', () => { + const recommendedIP = getRecommendedSubnetIPv4('172.16.0.0/16', []); + expect(recommendedIP).toEqual('172.16.1.0/24'); + }); +}); diff --git a/packages/manager/src/utilities/subnets.ts b/packages/manager/src/utilities/subnets.ts index d53ff5a5774..072e1518865 100644 --- a/packages/manager/src/utilities/subnets.ts +++ b/packages/manager/src/utilities/subnets.ts @@ -69,38 +69,43 @@ 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))) - ) { - return undefined; - } + parsedSecondOctet >= 16 && + parsedSecondOctet <= 31 && + parsedMask >= 12) || + // check for valid 192.x IPs + (firstOctet === '192' && secondOctet === '168' && parsedMask >= 16) + ); +}; - return SubnetMaskToAvailIPv4s[mask]; +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; }; /** @@ -109,8 +114,8 @@ export const calculateAvailableIPv4sRFC1918 = ( * @param otherIPv4s the other IPv4s to check against * @returns the next recommended subnet IPv4 address to use * - * Assumption: if the inputted ipv4 is valid and in x.x.x.x/x format, then the outputted ipv4 valid and in x.x.x.x/x format not already in - * otherIPv4s (excluding the default case -- see comments below) + * 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). */ export const getRecommendedSubnetIPv4 = ( lastRecommendedIPv4: string, @@ -120,22 +125,29 @@ export const getRecommendedSubnetIPv4 = ( firstOctet, secondOctet, thirdOctet, - fourthOctet, + fourthOctetAndMask, ] = lastRecommendedIPv4.split('.'); + const [fourthOctet] = fourthOctetAndMask.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 are plenty of subnets to recommend + * 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 (isNaN(parsedThirdOctet) || parsedThirdOctet + 1 > 255) { + if ( + !isValidRFC1918IPv4(lastRecommendedIPv4) || + isNaN(parsedThirdOctet) || + parsedThirdOctet + 1 > 255 + ) { return DEFAULT_SUBNET_IPV4_VALUE; } else { + // Automatically adding on a /24 mask to avoid situations where an invalid IP is recommended + // For example: 172.16.0.0/ ipv4ToReturn = `${firstOctet}.${secondOctet}.${ parsedThirdOctet + 1 - }.${fourthOctet}`; + }.${fourthOctet}/24`; } if (otherIPv4s.some((ip) => ip === ipv4ToReturn)) { From 4b5df853c740a287dba2ff8593cfeacc1a47f9a5 Mon Sep 17 00:00:00 2001 From: coliu-akamai Date: Tue, 19 Dec 2023 20:20:45 -0500 Subject: [PATCH 4/8] changeset and switch SubnetCreateDrawer logic --- .../.changeset/pr-10010-added-1703035008158.md | 5 +++++ .../features/VPCs/VPCDetail/SubnetCreateDrawer.tsx | 13 ++++--------- 2 files changed, 9 insertions(+), 9 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/VPCDetail/SubnetCreateDrawer.tsx b/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx index 77587af55ac..c0e24fb81b5 100644 --- a/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx +++ b/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx @@ -31,15 +31,10 @@ export const SubnetCreateDrawer = (props: Props) => { const userCannotAddSubnet = profile?.restricted && !grants?.global.add_vpcs; - const recommendedIPv4 = - allSubnets && allSubnets.length > 0 - ? getRecommendedSubnetIPv4( - // for simplicity purposes, we take the IPv4 of the last subnet in allSubnets, as - // getRecommendedSubnetIPv4 will iterate through potential IPv4s to recommend anyway - allSubnets[allSubnets.length - 1].ipv4 ?? '', - allSubnets.map((subnet) => subnet.ipv4 ?? '') - ) - : DEFAULT_SUBNET_IPV4_VALUE; + const recommendedIPv4 = getRecommendedSubnetIPv4( + DEFAULT_SUBNET_IPV4_VALUE, + allSubnets?.map((subnet) => subnet.ipv4 ?? '') ?? [] + ); const [errorMap, setErrorMap] = React.useState< Record From ccb87c24f8c6b43b5a6777fcc6be7d687f4f6b03 Mon Sep 17 00:00:00 2001 From: coliu-akamai Date: Tue, 19 Dec 2023 20:32:18 -0500 Subject: [PATCH 5/8] update comments and tests --- packages/manager/src/utilities/subnets.test.ts | 4 ++-- packages/manager/src/utilities/subnets.ts | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/manager/src/utilities/subnets.test.ts b/packages/manager/src/utilities/subnets.test.ts index d20cc17d767..03a4a6de8ee 100644 --- a/packages/manager/src/utilities/subnets.test.ts +++ b/packages/manager/src/utilities/subnets.test.ts @@ -121,8 +121,8 @@ describe('getRecommendedSubnetIPv4', () => { expect(recommendedIP).toEqual('10.0.7.0/24'); }); - it('recommends IPs with a /24 mask', () => { + 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/24'); + 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 072e1518865..db66e253b93 100644 --- a/packages/manager/src/utilities/subnets.ts +++ b/packages/manager/src/utilities/subnets.ts @@ -109,13 +109,16 @@ export const calculateAvailableIPv4sRFC1918 = ( }; /** - * Calculates the next subnet IPv4 address to recommend when creating a subnet, based off of the last recommended ipv4 and already existing IPv4s. + * 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). + * 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, @@ -125,9 +128,8 @@ export const getRecommendedSubnetIPv4 = ( firstOctet, secondOctet, thirdOctet, - fourthOctetAndMask, + fourthOctet, ] = lastRecommendedIPv4.split('.'); - const [fourthOctet] = fourthOctetAndMask.split('/'); const parsedThirdOctet = parseInt(thirdOctet, 10); let ipv4ToReturn = ''; @@ -143,11 +145,9 @@ export const getRecommendedSubnetIPv4 = ( ) { return DEFAULT_SUBNET_IPV4_VALUE; } else { - // Automatically adding on a /24 mask to avoid situations where an invalid IP is recommended - // For example: 172.16.0.0/ ipv4ToReturn = `${firstOctet}.${secondOctet}.${ parsedThirdOctet + 1 - }.${fourthOctet}/24`; + }.${fourthOctet}`; } if (otherIPv4s.some((ip) => ip === ipv4ToReturn)) { From 94c6b7e2cadbda3a1c7e84ac9f1c7a9485905189 Mon Sep 17 00:00:00 2001 From: coliu-akamai Date: Wed, 20 Dec 2023 00:40:52 -0500 Subject: [PATCH 6/8] fix some potential issues --- packages/manager/src/utilities/subnets.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/manager/src/utilities/subnets.ts b/packages/manager/src/utilities/subnets.ts index db66e253b93..30463ece370 100644 --- a/packages/manager/src/utilities/subnets.ts +++ b/packages/manager/src/utilities/subnets.ts @@ -150,7 +150,13 @@ export const getRecommendedSubnetIPv4 = ( }.${fourthOctet}`; } - if (otherIPv4s.some((ip) => ip === ipv4ToReturn)) { + if ( + otherIPv4s.some((ip) => { + const [_ip] = ip.split('/'); + const [_ipv4ToReturn] = ipv4ToReturn.split('/'); + return ip === ipv4ToReturn || _ip === _ipv4ToReturn; + }) + ) { return getRecommendedSubnetIPv4(ipv4ToReturn, otherIPv4s); } From f8d7aa2ee18498000f88893ce97e06d9a22386a4 Mon Sep 17 00:00:00 2001 From: coliu-akamai Date: Wed, 20 Dec 2023 09:32:10 -0500 Subject: [PATCH 7/8] update logic a bit --- packages/manager/src/utilities/subnets.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/manager/src/utilities/subnets.ts b/packages/manager/src/utilities/subnets.ts index 30463ece370..44ad6d4449a 100644 --- a/packages/manager/src/utilities/subnets.ts +++ b/packages/manager/src/utilities/subnets.ts @@ -150,11 +150,12 @@ export const getRecommendedSubnetIPv4 = ( }.${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 || _ip === _ipv4ToReturn; + return _ip === _ipv4ToReturn; }) ) { return getRecommendedSubnetIPv4(ipv4ToReturn, otherIPv4s); From 644f882aa7263eab8c265f9c6d6465ed30f8da66 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Tue, 2 Jan 2024 15:33:28 -0500 Subject: [PATCH 8/8] use `useVPCQuery` to get all subnets insted of extra query --- .../features/VPCs/VPCDetail/SubnetCreateDrawer.tsx | 6 +++--- packages/manager/src/queries/vpcs.ts | 14 -------------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx b/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx index c0e24fb81b5..d7e613b0157 100644 --- a/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx +++ b/packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx @@ -6,7 +6,7 @@ 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 { useAllSubnetsQuery, useCreateSubnetMutation } from 'src/queries/vpcs'; +import { useCreateSubnetMutation, useVPCQuery } from 'src/queries/vpcs'; import { getErrorMap } from 'src/utilities/errorUtils'; import { DEFAULT_SUBNET_IPV4_VALUE, @@ -27,13 +27,13 @@ export const SubnetCreateDrawer = (props: Props) => { const { data: profile } = useProfile(); const { data: grants } = useGrants(); - const { data: allSubnets } = useAllSubnetsQuery(vpcId); + const { data: vpc } = useVPCQuery(vpcId, open); const userCannotAddSubnet = profile?.restricted && !grants?.global.add_vpcs; const recommendedIPv4 = getRecommendedSubnetIPv4( DEFAULT_SUBNET_IPV4_VALUE, - allSubnets?.map((subnet) => subnet.ipv4 ?? '') ?? [] + vpc?.subnets?.map((subnet) => subnet.ipv4 ?? '') ?? [] ); const [errorMap, setErrorMap] = React.useState< diff --git a/packages/manager/src/queries/vpcs.ts b/packages/manager/src/queries/vpcs.ts index 3693b9bd9a1..10175e32a41 100644 --- a/packages/manager/src/queries/vpcs.ts +++ b/packages/manager/src/queries/vpcs.ts @@ -24,8 +24,6 @@ import { } from '@linode/api-v4/lib/types'; import { useMutation, useQuery, useQueryClient } from 'react-query'; -import { getAll } from 'src/utilities/getAll'; - export const vpcQueryKey = 'vpcs'; export const subnetQueryKey = 'subnets'; @@ -98,18 +96,6 @@ export const useSubnetsQuery = ( ); }; -export const useAllSubnetsQuery = (vpcID: number, enabled: boolean = true) => - useQuery( - [vpcQueryKey, 'vpc', vpcID, subnetQueryKey, 'unpaginated'], - () => getAllSubnetsRequest(vpcID), - { enabled, keepPreviousData: true } - ); - -const getAllSubnetsRequest = (vpcID: number) => - getAll((params, filters) => - getSubnets(vpcID, params, filters) - )().then((data) => data.data); - export const useSubnetQuery = (vpcID: number, subnetID: number) => { return useQuery( [vpcQueryKey, 'vpc', vpcID, subnetQueryKey, 'subnet', subnetID],