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.

Sign up for GitHub

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-7645] – Support VPC IPv4 Ranges in Add/Edit Linode Config dialog #10170

Merged
merged 5 commits into from
Feb 12, 2024
Merged

feat: [M3-7645] – Support VPC IPv4 Ranges in Add/Edit Linode Config dialog #10170

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4239c58
Support VPC IPv4 Ranges in Add/Edit Linode Config dialog
DevDW Feb 9, 2024
934c0b7
Added changeset: Support VPC IPv4 Ranges in Add/Edit Linode Config di…
DevDW Feb 9, 2024
42fd6f2
Merge in latest staging-off-release and resolve conflict
DevDW Feb 12, 2024
48ec35b
Merge branch 'staging-off-release' into M3-7645-clean-ip-ranges-confi…
DevDW Feb 12, 2024
f6f8ea3
Update changelog and remove changeset
DevDW Feb 12, 2024
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-10170-added-1707506171709.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Added
---

Support VPC IPv4 Ranges in Add/Edit Linode Config dialog ([#10170](https://github.com/linode/manager/pull/10170))
2 changes: 1 addition & 1 deletion packages/manager/src/features/Linodes/LinodesCreate/LinodeCreate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import { doesRegionSupportFeature } from 'src/utilities/doesRegionSupportFeature
import { getErrorMap } from 'src/utilities/errorUtils';
import { extendType } from 'src/utilities/extendType';
import { filterCurrentTypes } from 'src/utilities/filterCurrentLinodeTypes';
import { ExtendedIP } from 'src/utilities/ipUtils';
import { getMonthlyBackupsPrice } from 'src/utilities/pricing/backups';
import { UNKNOWN_PRICE } from 'src/utilities/pricing/constants';
import { renderMonthlyPriceToCorrectDecimalPlace } from 'src/utilities/pricing/dynamicPricing';
Expand Down Expand Up @@ -99,7 +100,6 @@ import {
} from './types';

import type { Tab } from 'src/components/Tabs/TabLinkList';
import { ExtendedIP } from 'src/utilities/ipUtils';

export interface LinodeCreateProps {
additionalIPv4RangesForVPC: ExtendedIP[];
Expand Down
6 changes: 4 additions & 2 deletions packages/manager/src/features/Linodes/LinodesCreate/VPCPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export interface VPCPanelProps {
subnetError?: string;
toggleAssignPublicIPv4Address: () => void;
toggleAutoassignIPv4WithinVPCEnabled: () => void;
vpcIPRangesError?: string;
vpcIPv4AddressOfLinode: string | undefined;
vpcIPv4Error?: string;
vpcIdError?: string;
Expand All @@ -69,6 +70,7 @@ export const VPCPanel = (props: VPCPanelProps) => {
subnetError,
toggleAssignPublicIPv4Address,
toggleAutoassignIPv4WithinVPCEnabled,
vpcIPRangesError,
vpcIPv4AddressOfLinode,
vpcIPv4Error,
vpcIdError,
Expand Down Expand Up @@ -347,9 +349,9 @@ export const VPCPanel = (props: VPCPanelProps) => {
)}
<AssignIPRanges
handleIPRangeChange={handleIPv4RangeChange}
includeDescriptionInTooltip={fromLinodeConfig}
ipRanges={additionalIPv4RangesForVPC}
ipRangesError={''}
sx={{}}
ipRangesError={vpcIPRangesError}
/>
</>
)}
Expand Down
96 changes: 50 additions & 46 deletions packages/manager/src/features/Linodes/LinodesDetail/LinodeConfigs/LinodeConfigDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ import {
NATTED_PUBLIC_IP_HELPER_TEXT,
NOT_NATTED_HELPER_TEXT,
} from 'src/features/VPCs/constants';
import { useAccountManagement } from 'src/hooks/useAccountManagement';
import { useFlags } from 'src/hooks/useFlags';
import {
useLinodeConfigCreateMutation,
useLinodeConfigUpdateMutation,
Expand All @@ -53,7 +51,6 @@ import { useRegionsQuery } from 'src/queries/regions';
import { queryKey as vlansQueryKey } from 'src/queries/vlans';
import { useAllVolumesQuery } from 'src/queries/volumes';
import { vpcQueryKey } from 'src/queries/vpcs';
import { isFeatureEnabled } from 'src/utilities/accountCapabilities';
import {
DevicesAsStrings,
createDevicesFromStrings,
Expand All @@ -64,6 +61,7 @@ import {
handleGeneralErrors,
} from 'src/utilities/formikErrorUtils';
import { getSelectedOptionFromGroupedOptions } from 'src/utilities/getSelectedOptionFromGroupedOptions';
import { ExtendedIP } from 'src/utilities/ipUtils';
import { scrollErrorIntoView } from 'src/utilities/scrollErrorIntoView';

import {
Expand Down Expand Up @@ -116,6 +114,7 @@ interface Props {
}

const defaultInterface = {
ip_ranges: [],
ipam_address: '',
label: '',
purpose: 'none',
Expand Down Expand Up @@ -183,6 +182,7 @@ const interfacesToState = (interfaces?: Interface[]) => {
const interfacesPayload = interfaces.map(
({
id,
ip_ranges,
ipam_address,
ipv4,
label,
Expand All @@ -192,6 +192,7 @@ const interfacesToState = (interfaces?: Interface[]) => {
vpc_id,
}) => ({
id,
ip_ranges,
ipam_address,
ipv4,
label,
Expand Down Expand Up @@ -262,8 +263,6 @@ export const LinodeConfigDialog = (props: Props) => {
);

const theme = useTheme();
const flags = useFlags();
const { account } = useAccountManagement();

const regions = useRegionsQuery().data ?? [];

Expand Down Expand Up @@ -291,13 +290,6 @@ export const LinodeConfigDialog = (props: Props) => {
thisRegion.capabilities.includes('VPCs')
);

// @TODO VPC: remove once VPC is fully rolled out
const vpcEnabled = isFeatureEnabled(
'VPCs',
Boolean(flags.vpc),
account?.capabilities ?? []
);

const { resetForm, setFieldValue, values, ...formik } = useFormik({
initialValues: defaultFieldsValues,
onSubmit: (values) => onSubmit(values),
Expand Down Expand Up @@ -493,7 +485,7 @@ export const LinodeConfigDialog = (props: Props) => {
(_interface) => _interface.primary === true
);

if (vpcEnabled && indexOfExistingPrimaryInterface !== -1) {
if (indexOfExistingPrimaryInterface !== -1) {
setPrimaryInterfaceIndex(indexOfExistingPrimaryInterface);
}

Expand Down Expand Up @@ -523,7 +515,7 @@ export const LinodeConfigDialog = (props: Props) => {
setPrimaryInterfaceIndex(0);
}
}
}, [open, config, initrdFromConfig, resetForm, queryClient, vpcEnabled]);
}, [open, config, initrdFromConfig, resetForm, queryClient]);

const generalError = formik.status?.generalError;

Expand Down Expand Up @@ -667,12 +659,9 @@ export const LinodeConfigDialog = (props: Props) => {

const networkInterfacesHelperText = (
<Typography>
Configure the network that a selected interface will connect to (
{vpcEnabled
? '"Public Internet", VLAN, or VPC'
: 'either "Public Internet" or a VLAN'}
) . Each Linode can have up to three Network Interfaces. For more
information, see our{' '}
Configure the network that a selected interface will connect to
&quot;Public Internet&quot;, VLAN, or VPC. Each Linode can have up to
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the &quot;?

Suggested change
&quot;Public Internet&quot;, VLAN, or VPC. Each Linode can have up to
"Public Internet", VLAN, or VPC. Each Linode can have up to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ESLint recommended escaping them

Screenshot 2024-02-12 at 12 34 48 PM

three Network Interfaces. For more information, see our{' '}
<Link to="https://www.linode.com/docs/products/networking/vlans/guides/attach-to-compute-instance/#attaching-a-vlan-to-an-existing-compute-instance">
Network Interfaces guide
</Link>
Expand Down Expand Up @@ -951,9 +940,7 @@ export const LinodeConfigDialog = (props: Props) => {

<Grid xs={12}>
<Box alignItems="center" display="flex">
<Typography variant="h3">
{vpcEnabled ? 'Networking' : 'Network Interfaces'}
</Typography>
<Typography variant="h3">Networking</Typography>
<TooltipIcon
sxTooltipIcon={{
paddingBottom: 0,
Expand All @@ -971,30 +958,44 @@ export const LinodeConfigDialog = (props: Props) => {
variant="error"
/>
)}
{vpcEnabled && (
<>
<Select
defaultValue={
primaryInterfaceOptions[primaryInterfaceIndex ?? 0]
}
data-testid="primary-interface-dropdown"
disabled={isReadOnly}
isClearable={false}
label="Primary Interface (Default Route)"
onChange={handlePrimaryInterfaceChange}
options={getPrimaryInterfaceOptions(values.interfaces)}
/>
<Divider
sx={{
margin: `${theme.spacing(
4.5
)} ${theme.spacing()} ${theme.spacing(1.5)} `,
width: `calc(100% - ${theme.spacing(2)})`,
}}
/>
</>
)}
<>
<Select
defaultValue={
primaryInterfaceOptions[primaryInterfaceIndex ?? 0]
}
data-testid="primary-interface-dropdown"
disabled={isReadOnly}
isClearable={false}
label="Primary Interface (Default Route)"
onChange={handlePrimaryInterfaceChange}
options={getPrimaryInterfaceOptions(values.interfaces)}
/>
<Divider
sx={{
margin: `${theme.spacing(
4.5
)} ${theme.spacing()} ${theme.spacing(1.5)} `,
width: `calc(100% - ${theme.spacing(2)})`,
}}
/>
</>
{values.interfaces.map((thisInterface, idx) => {
const thisInterfaceIPRanges: ExtendedIP[] = (
thisInterface.ip_ranges ?? []
).map((ip_range, index) => {
// Display a more user-friendly error to the user as opposed to, for example, "interfaces[1].ip_ranges[1] is invalid"
const errorString: string = formik.errors[
`interfaces[${idx}].ip_ranges[${index}]`
]?.includes('is invalid')
? 'Invalid IP range'
: formik.errors[`interfaces[${idx}].ip_ranges[${index}]`];

return {
address: ip_range,
error: errorString,
};
});
Comment on lines +983 to +997
Copy link
Contributor Author

@dwiley-akamai dwiley-akamai Feb 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Because these are deeply nested errors that we're trying to surface, it required a bit of less-than-ideal logic and mapping.

I am aware of one unfortunate shortcoming–if you have multiple IP ranges but one of them (not the last one in the sequence) gets an error when you click "Save Changes", clicking the "X" to remove that IP range will result in the error moving onto the range that moves up to occupy that index. Sadly, I could not see a way around this, as I tried having something like ipRangeIndicesWithErrors in component state or as a ref to track the indices so that the errors could be cleared from inside handleInterfaceChange or handleChange, but that was not fruitful.


return (
<React.Fragment key={`${idx}-interface`}>
{unrecommendedConfigNoticeSelector({
Expand All @@ -1005,6 +1006,8 @@ export const LinodeConfigDialog = (props: Props) => {
})}
<InterfaceSelect
errors={{
ipRangeError:
formik.errors[`interfaces[${idx}].ip_ranges`],
ipamError:
formik.errors[`interfaces[${idx}].ipam_address`],
labelError: formik.errors[`interfaces[${idx}].label`],
Expand All @@ -1021,6 +1024,7 @@ export const LinodeConfigDialog = (props: Props) => {
handleChange={(newInterface: Interface) =>
handleInterfaceChange(idx, newInterface)
}
additionalIPv4RangesForVPC={thisInterfaceIPRanges}
ipamAddress={thisInterface.ipam_address}
key={`eth${idx}-interface`}
label={thisInterface.label}
Expand Down
29 changes: 27 additions & 2 deletions packages/manager/src/features/Linodes/LinodesDetail/LinodeSettings/InterfaceSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ import { useAccount } from 'src/queries/account';
import { useVlansQuery } from 'src/queries/vlans';
import { isFeatureEnabled } from 'src/utilities/accountCapabilities';
import { sendLinodeCreateDocsEvent } from 'src/utilities/analytics';
import { ExtendedIP } from 'src/utilities/ipUtils';

interface Props {
additionalIPv4RangesForVPC?: ExtendedIP[];
errors: VPCInterfaceErrors & OtherInterfaceErrors;
fromAddonsPanel?: boolean;
handleChange: (updatedInterface: ExtendedInterface) => void;
Expand All @@ -33,6 +35,7 @@ interface Props {
slotNumber: number;
}
interface VPCInterfaceErrors {
ipRangeError?: string;
labelError?: string;
publicIPv4Error?: string;
subnetError?: string;
Expand Down Expand Up @@ -63,6 +66,7 @@ type CombinedProps = Props & VPCState;

export const InterfaceSelect = (props: CombinedProps) => {
const {
additionalIPv4RangesForVPC,
errors,
fromAddonsPanel,
handleChange,
Expand Down Expand Up @@ -118,6 +122,9 @@ export const InterfaceSelect = (props: CombinedProps) => {
const [autoAssignLinodeIPv4, setAutoAssignLinodeIPv4] = React.useState(
Boolean(nattedIPv4Address)
);
const _additionalIPv4RangesForVPC = additionalIPv4RangesForVPC?.map(
(ip_range) => ip_range.address
);

const handlePurposeChange = (selected: Item<InterfacePurpose>) => {
const purpose = selected.value;
Expand All @@ -142,6 +149,7 @@ export const InterfaceSelect = (props: CombinedProps) => {
// Only clear VPC related fields if VPC selection changes
if (selectedVPCId !== vpcId) {
handleChange({
ip_ranges: _additionalIPv4RangesForVPC,
ipam_address: null,
ipv4: {
nat_1_1: autoAssignLinodeIPv4 ? 'any' : undefined,
Expand All @@ -155,8 +163,22 @@ export const InterfaceSelect = (props: CombinedProps) => {
}
};

const handleIPv4RangeChange = (ipv4Ranges: ExtendedIP[]) => {
const changeObj = {
ip_ranges: ipv4Ranges.map((ip_range) => ip_range.address),
ipam_address: null,
label: null,
purpose,
subnet_id: subnetId,
vpc_id: vpcId,
};

handleChange(changeObj);
};

const handleSubnetChange = (selectedSubnetId: number) =>
handleChange({
ip_ranges: _additionalIPv4RangesForVPC,
ipam_address: null,
ipv4: {
nat_1_1: autoAssignLinodeIPv4 ? 'any' : undefined,
Expand All @@ -170,6 +192,7 @@ export const InterfaceSelect = (props: CombinedProps) => {

const handleVPCIPv4Input = (vpcIPv4Input: string) => {
const changeObj = {
ip_ranges: _additionalIPv4RangesForVPC,
ipam_address: null,
label: null,
purpose,
Expand Down Expand Up @@ -204,6 +227,7 @@ export const InterfaceSelect = (props: CombinedProps) => {
}

const changeObj = {
ip_ranges: _additionalIPv4RangesForVPC,
ipam_address: null,
label: null,
purpose,
Expand Down Expand Up @@ -386,11 +410,11 @@ export const InterfaceSelect = (props: CombinedProps) => {
toggleAutoassignIPv4WithinVPCEnabled={() =>
setAutoAssignVPCIPv4((autoAssignVPCIPv4) => !autoAssignVPCIPv4)
}
additionalIPv4RangesForVPC={[]} // @TODO VPC: temporary placeholder to before M3-7645 is worked on to prevent errors
additionalIPv4RangesForVPC={additionalIPv4RangesForVPC ?? []}
assignPublicIPv4Address={autoAssignLinodeIPv4}
autoassignIPv4WithinVPC={autoAssignVPCIPv4}
from="linodeConfig"
handleIPv4RangeChange={() => null} // @TODO VPC: temporary placeholder to before M3-7645 is worked on to prevent errors
handleIPv4RangeChange={handleIPv4RangeChange}
handleSelectVPC={handleVPCLabelChange}
handleSubnetChange={handleSubnetChange}
handleVPCIPv4Change={handleVPCIPv4Input}
Expand All @@ -399,6 +423,7 @@ export const InterfaceSelect = (props: CombinedProps) => {
selectedSubnetId={subnetId}
selectedVPCId={vpcId}
subnetError={errors.subnetError}
vpcIPRangesError={errors.ipRangeError}
vpcIPv4AddressOfLinode={vpcIPv4}
vpcIPv4Error={errors.vpcIPv4Error}
vpcIdError={errors.vpcError}
Expand Down
Loading
Loading