Skip to content

Commit

Permalink
feat: [M3-8560] - NodeBalancer Configurations - Support Linodes with …
Browse files Browse the repository at this point in the history
…Multiple Private IPs (#11069)

* initial work to support multiple private ipv4s on a Linode

* fix style bug

* clean up validation and utils

* add changeset

* add more detailed changesets

* feedback @hkhalil-akamai

* clean `LinodeSelect` extra props

---------

Co-authored-by: Banks Nussman <banks@nussman.us>
  • Loading branch information
bnussman-akamai and bnussman authored Oct 11, 2024
1 parent 150d2e5 commit 60a1925
Show file tree
Hide file tree
Showing 16 changed files with 204 additions and 165 deletions.
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-11069-fixed-1728443895478.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Fixed
---

Support Linodes with multiple private IPs in NodeBalancer configurations ([#11069](https://github.com/linode/manager/pull/11069))
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,4 +1,3 @@
import { Linode } from '@linode/api-v4';
import { screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import React from 'react';
Expand All @@ -8,49 +7,12 @@ import { renderWithTheme } from 'src/utilities/testHelpers';

import { LinodeSelect } from './LinodeSelect';

const fakeLinodeData = linodeFactory.build({
id: 1,
image: 'metadata-test-image',
label: 'metadata-test-region',
region: 'eu-west',
});
import type { Linode } from '@linode/api-v4';


const TEXTFIELD_ID = 'textfield-input';

describe('LinodeSelect', () => {
test('renders custom options using renderOption', async () => {
// Create a mock renderOption function
const mockRenderOption = (linode: Linode, selected: boolean) => (
<span data-testid={`custom-option-${linode.id}`}>
{`${linode.label} - ${selected ? 'Selected' : 'Not Selected'}`}
</span>
);

// Render the component with the custom renderOption function
renderWithTheme(
<LinodeSelect
multiple={false}
onSelectionChange={vi.fn()} // Placeholder, as there's no callback
options={[fakeLinodeData]}
renderOption={mockRenderOption}
value={null}
/>
);

const input = screen.getByTestId(TEXTFIELD_ID);

// Open the dropdown
await userEvent.click(input);

// Wait for the options to load (use some unique identifier for the options)
await waitFor(() => {
const customOption = screen.getByTestId('custom-option-1');
expect(customOption).toBeInTheDocument();
expect(customOption).toHaveTextContent(
'metadata-test-region - Not Selected'
);
});
});
test('should display custom no options message', async () => {
const customNoOptionsMessage = 'Custom No Options Message';
const options: Linode[] = []; // Assuming no options are available
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@ interface LinodeSelectProps {
optionsFilter?: (linode: Linode) => boolean;
/* Displayed when the input is blank. */
placeholder?: string;
/* Render a custom option. */
renderOption?: (linode: Linode, selected: boolean) => JSX.Element;
/* Render a custom option label. */
renderOptionLabel?: (linode: Linode) => string;
/* Displays an indication that the input is required. */
required?: boolean;
/* Adds custom styles to the component. */
Expand Down Expand Up @@ -98,8 +94,6 @@ export const LinodeSelect = (
options,
optionsFilter,
placeholder,
renderOption,
renderOptionLabel,
sx,
value,
} = props;
Expand All @@ -122,9 +116,6 @@ export const LinodeSelect = (

return (
<Autocomplete
getOptionLabel={(linode: Linode) =>
renderOptionLabel ? renderOptionLabel(linode) : linode.label
}
isOptionEqualToValue={
checkIsOptionEqualToValue
? (option, value) => option.id === value.id
Expand All @@ -145,17 +136,6 @@ export const LinodeSelect = (
? 'Select Linodes'
: 'Select a Linode'
}
renderOption={
renderOption
? (props, option, { selected }) => {
return (
<li {...props} data-qa-linode-option>
{renderOption(option, selected)}
</li>
);
}
: undefined
}
value={
typeof value === 'function'
? multiple && Array.isArray(value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ 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 { isPrivateIP } from 'src/utilities/ipUtils';
import { tail } from 'src/utilities/tail';

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

export const sortIPAddress = (ip1: string, ip2: string) =>
(privateIPRegex.test(ip1) ? 1 : -1) - (privateIPRegex.test(ip2) ? 1 : -1);
(isPrivateIP(ip1) ? 1 : -1) - (isPrivateIP(ip2) ? 1 : -1);

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
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)) ?? ''
}
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}
/>
);
});
Loading

0 comments on commit 60a1925

Please sign in to comment.