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-8560] - NodeBalancer Configurations - Support Linodes with Multiple Private IPs #11069

Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { useOrder } from 'src/hooks/useOrder';
import { usePagination } from 'src/hooks/usePagination';
import { useLinodesQuery } from 'src/queries/linodes/linodes';
import { sendLinodePowerOffEvent } from 'src/utilities/analytics/customEventAnalytics';
import { privateIPRegex } from 'src/utilities/ipUtils';
import { isPrivateIP } from 'src/utilities/ipUtils';
import { isNumeric } from 'src/utilities/stringUtils';

import {
Expand Down Expand Up @@ -105,7 +105,7 @@ export const LinodeSelectTable = (props: Props) => {
const queryClient = useQueryClient();

const handleSelect = async (linode: Linode) => {
const hasPrivateIP = linode.ipv4.some((ipv4) => privateIPRegex.test(ipv4));
const hasPrivateIP = linode.ipv4.some(isPrivateIP);
reset((prev) => ({
...prev,
backup_id: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { linodeQueries } from 'src/queries/linodes/linodes';
import { stackscriptQueries } from 'src/queries/stackscripts';
import { sendCreateLinodeEvent } from 'src/utilities/analytics/customEventAnalytics';
import { sendLinodeCreateFormErrorEvent } from 'src/utilities/analytics/formEventAnalytics';
import { privateIPRegex } from 'src/utilities/ipUtils';
import { isPrivateIP } from 'src/utilities/ipUtils';
import { utoa } from 'src/utilities/metadata';
import { isNotNullOrUndefined } from 'src/utilities/nullOrUndefined';
import { omitProps } from 'src/utilities/omittedProps';
Expand Down Expand Up @@ -299,8 +299,7 @@ export const defaultValues = async (
? await queryClient.ensureQueryData(linodeQueries.linode(params.linodeID))
: null;

const privateIp =
linode?.ipv4.some((ipv4) => privateIPRegex.test(ipv4)) ?? false;
const privateIp = linode?.ipv4.some(isPrivateIP) ?? false;

const values: LinodeCreateFormValues = {
backup_id: params.backupID,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { PRIVATE_IPv4_REGEX } from '@linode/validation';
import * as React from 'react';

import { CopyTooltip } from 'src/components/CopyTooltip/CopyTooltip';
import { ShowMore } from 'src/components/ShowMore/ShowMore';
import { PublicIpsUnassignedTooltip } from 'src/features/Linodes/PublicIpsUnassignedTooltip';
import { privateIPRegex } from 'src/utilities/ipUtils';
import { tail } from 'src/utilities/tail';

import {
Expand Down Expand Up @@ -55,7 +55,8 @@ export interface IPAddressProps {
}

export const sortIPAddress = (ip1: string, ip2: string) =>
(privateIPRegex.test(ip1) ? 1 : -1) - (privateIPRegex.test(ip2) ? 1 : -1);
(PRIVATE_IPv4_REGEX.test(ip1) ? 1 : -1) -
(PRIVATE_IPv4_REGEX.test(ip2) ? 1 : -1);
bnussman-akamai marked this conversation as resolved.
Show resolved Hide resolved

export const IPAddress = (props: IPAddressProps) => {
const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
handleFieldErrors,
handleGeneralErrors,
} from 'src/utilities/formikErrorUtils';
import { privateIPRegex, removePrefixLength } from 'src/utilities/ipUtils';
import { isPrivateIP, removePrefixLength } from 'src/utilities/ipUtils';

import {
StyledIPGrid,
Expand Down Expand Up @@ -168,9 +168,7 @@ const EditSSHAccessDrawer = (props: EditSSHAccessDrawerProps) => {
},
...options
// Remove Private IPs
.filter(
(option) => !privateIPRegex.test(option.value)
)
.filter((option) => !isPrivateIP(option.value))
// Remove the prefix length from each option.
.map((option) => ({
label: removePrefixLength(option.value),
Expand Down
170 changes: 88 additions & 82 deletions packages/manager/src/features/NodeBalancers/ConfigNodeIPSelect.tsx
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary changes are here 🚨

Original file line number Diff line number Diff line change
@@ -1,96 +1,102 @@
import { Box } from '@mui/material';
import * as React from 'react';
import React from 'react';

import { Autocomplete } from 'src/components/Autocomplete/Autocomplete';
import { SelectedIcon } from 'src/components/Autocomplete/Autocomplete.styles';
import { LinodeSelect } from 'src/features/Linodes/LinodeSelect/LinodeSelect';
import { privateIPRegex } from 'src/utilities/ipUtils';
import { Box } from 'src/components/Box';
import { Stack } from 'src/components/Stack';
import { Typography } from 'src/components/Typography';
import { useAllLinodesQuery } from 'src/queries/linodes/linodes';

import type { Linode } from '@linode/api-v4/lib/linodes';
import type { TextFieldProps } from 'src/components/TextField';
import { getPrivateIPOptions } from './ConfigNodeIPSelect.utils';

interface ConfigNodeIPSelectProps {
interface Props {
/**
* Disables the select
*/
disabled?: boolean;
errorText?: string;
/**
* Validation error text
*/
errorText: string | undefined;
/**
* Function that is called when the select's value changes
*/
handleChange: (nodeIndex: number, ipAddress: null | string) => void;
/**
* Override the default input `id` for the select
*/
inputId?: string;
nodeAddress?: string;
/**
* The selected private IP address
*/
nodeAddress: string | undefined;
/**
* The index of the config node in state
*/
nodeIndex: number;
selectedRegion?: string;
textfieldProps: Omit<TextFieldProps, 'label'>;
/**
* The region for which to load Linodes and to show private IPs
* @note IPs won't load until a region is passed
*/
region: string | undefined;
}

export const ConfigNodeIPSelect = React.memo(
(props: ConfigNodeIPSelectProps) => {
const {
handleChange: _handleChange,
inputId,
nodeAddress,
nodeIndex,
} = props;

const handleChange = (linode: Linode | null) => {
if (!linode) {
_handleChange(nodeIndex, null);
}
export const ConfigNodeIPSelect = React.memo((props: Props) => {
const {
disabled,
errorText,
handleChange,
inputId,
nodeAddress,
nodeIndex,
region,
} = props;

const thisLinodesPrivateIP = linode?.ipv4.find((ipv4) =>
ipv4.match(privateIPRegex)
);
const { data: linodes, error, isLoading } = useAllLinodesQuery(
{},
{ region },
region !== undefined
);

if (!thisLinodesPrivateIP) {
return;
}
const options = getPrivateIPOptions(linodes);

/**
* we can be sure the selection has a private IP because of the
* filterCondition prop in the render method below
*/
_handleChange(nodeIndex, thisLinodesPrivateIP);
};

return (
<LinodeSelect
noOptionsMessage={`No options - please ensure you have at least 1 Linode
with a private IP located in the selected region.`}
optionsFilter={(linode) => {
/**
* if the Linode doesn't have an private IP OR if the Linode
* is in a different region that the NodeBalancer, don't show it
* in the select dropdown
*/
return (
!!linode.ipv4.find((eachIP) => eachIP.match(privateIPRegex)) &&
linode.region === props.selectedRegion
);
}}
renderOption={(linode, selected) => (
<>
<Box
sx={{
flexGrow: 1,
}}
>
<strong>
{linode.ipv4.find((eachIP) => eachIP.match(privateIPRegex))}
</strong>
<div>{linode.label}</div>
</Box>
<SelectedIcon visible={selected} />
</>
)}
renderOptionLabel={(linode) =>
linode.ipv4.find((eachIP) => eachIP.match(privateIPRegex)) ?? ''
}
bnussman-akamai marked this conversation as resolved.
Show resolved Hide resolved
clearable
disabled={props.disabled}
errorText={props.errorText}
id={inputId}
label="IP Address"
noMarginTop
onSelectionChange={handleChange}
placeholder="Enter IP Address"
value={(linode) => linode.ipv4.some((ip) => ip === nodeAddress)}
/>
);
}
);
return (
<Autocomplete
renderOption={(props, option, { selected }) => (
<li {...props} key={props.key}>
<Box
alignItems="center"
display="flex"
flexDirection="row"
gap={1}
justifyContent="space-between"
width="100%"
>
<Stack>
<Typography
color="inherit"
fontFamily={(theme) => theme.font.bold}
>
{option.label}
</Typography>
<Typography color="inherit">{option.linode.label}</Typography>
</Stack>
{selected && <SelectedIcon visible />}
</Box>
</li>
)}
disabled={disabled}
errorText={errorText ?? error?.[0].reason}
id={inputId}
label="IP Address"
loading={isLoading}
noMarginTop
noOptionsText="No options - please ensure you have at least 1 Linode with a private IP located in the selected region."
onChange={(e, value) => handleChange(nodeIndex, value?.label ?? null)}
options={options}
placeholder="Enter IP Address"
value={options.find((o) => o.label === nodeAddress) ?? null}
/>
);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ Nice refactor

Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { linodeFactory } from 'src/factories';

import { getPrivateIPOptions } from './ConfigNodeIPSelect.utils';

describe('getPrivateIPOptions', () => {
it('returns an empty array when linodes are undefined', () => {
expect(getPrivateIPOptions(undefined)).toStrictEqual([]);
});

it('returns an empty array when there are no Linodes', () => {
expect(getPrivateIPOptions([])).toStrictEqual([]);
});

it('returns an option for each private IPv4 on a Linode', () => {
const linode = linodeFactory.build({ ipv4: ['192.168.1.1', '172.16.0.1'] });

expect(getPrivateIPOptions([linode])).toStrictEqual([
{ label: '192.168.1.1', linode },
{ label: '172.16.0.1', linode },
]);
});

it('does not return an option for public IPv4s on a Linode', () => {
const linode = linodeFactory.build({
ipv4: ['143.198.125.230', '192.168.1.1'],
});

expect(getPrivateIPOptions([linode])).toStrictEqual([
{ label: '192.168.1.1', linode },
]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { isPrivateIP } from 'src/utilities/ipUtils';

import type { Linode } from '@linode/api-v4';

interface PrivateIPOption {
/**
* A private IPv4 address
*/
label: string;
/**
* The Linode associated with the private IPv4 address
*/
linode: Linode;
}

/**
* Given an array of Linodes, this function returns an array of private
* IPv4 options intended to be used in a Select component.
*/
export const getPrivateIPOptions = (linodes: Linode[] | undefined) => {
if (!linodes) {
return [];
}

const options: PrivateIPOption[] = [];

for (const linode of linodes) {
for (const ip of linode.ipv4) {
if (isPrivateIP(ip)) {
options.push({ label: ip, linode });
}
}
}

return options;
};
Original file line number Diff line number Diff line change
Expand Up @@ -134,18 +134,13 @@ export const NodeBalancerConfigNode = React.memo(
<Grid container data-qa-node key={idx} spacing={2}>
<Grid lg={forEdit ? 2 : 4} sm={3} xs={12}>
<ConfigNodeIPSelect
textfieldProps={{
dataAttrs: {
'data-qa-backend-ip-address': true,
},
}}
disabled={disabled}
errorText={nodesErrorMap.address}
handleChange={onNodeAddressChange}
inputId={`ip-select-node-${configIdx}-${idx}`}
nodeAddress={node.address}
nodeIndex={idx}
selectedRegion={nodeBalancerRegion}
region={nodeBalancerRegion}
/>
</Grid>
<Grid lg={2} sm={3} xs={6}>
Expand Down
13 changes: 13 additions & 0 deletions packages/manager/src/utilities/ipUtils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { isPrivateIP } from './ipUtils';

describe('isPrivateIP', () => {
it('returns true for a private IPv4', () => {
expect(isPrivateIP('192.168.1.1')).toBe(true);
expect(isPrivateIP('172.16.5.12')).toBe(true);
});

it('returns false for a public IPv4', () => {
expect(isPrivateIP('45.79.245.236')).toBe(false);
expect(isPrivateIP('100.78.0.8')).toBe(false);
});
});
8 changes: 6 additions & 2 deletions packages/manager/src/utilities/ipUtils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { PRIVATE_IPv4_REGEX } from '@linode/validation';
import { parseCIDR, parse as parseIP } from 'ipaddr.js';

/**
Expand All @@ -8,9 +9,12 @@ import { parseCIDR, parse as parseIP } from 'ipaddr.js';
export const removePrefixLength = (ip: string) => ip.replace(/\/\d+/, '');

/**
* Regex for determining if a string is a private IP Addresses
* Determines if an IPv4 address is private
* @returns true if the given IPv4 address is private
*/
export const privateIPRegex = /^10\.|^172\.1[6-9]\.|^172\.2[0-9]\.|^172\.3[0-1]\.|^192\.168\.|^fd/;
export const isPrivateIP = (ip: string) => {
return PRIVATE_IPv4_REGEX.test(ip);
};

export interface ExtendedIP {
address: string;
Expand Down
Loading