diff --git a/packages/manager/.changeset/pr-9916-upcoming-features-1700499528271.md b/packages/manager/.changeset/pr-9916-upcoming-features-1700499528271.md new file mode 100644 index 00000000000..1ffb3f1cb5f --- /dev/null +++ b/packages/manager/.changeset/pr-9916-upcoming-features-1700499528271.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Upcoming Features +--- + +Display warning notices for unrecommended configurations in Linode Add/Edit Config dialog ([#9916](https://github.com/linode/manager/pull/9916)) diff --git a/packages/manager/src/features/Linodes/LinodesDetail/LinodeConfigs/LinodeConfigDialog.test.tsx b/packages/manager/src/features/Linodes/LinodesDetail/LinodeConfigs/LinodeConfigDialog.test.tsx index 0ae8fb3e5cf..d139fa5af59 100644 --- a/packages/manager/src/features/Linodes/LinodesDetail/LinodeConfigs/LinodeConfigDialog.test.tsx +++ b/packages/manager/src/features/Linodes/LinodesDetail/LinodeConfigs/LinodeConfigDialog.test.tsx @@ -1,14 +1,22 @@ import { waitFor } from '@testing-library/react'; import * as React from 'react'; -import { configFactory } from 'src/factories'; +import { LinodeConfigInterfaceFactory, configFactory } from 'src/factories'; import { accountFactory } from 'src/factories/account'; +import { + LINODE_UNREACHABLE_HELPER_TEXT, + NATTED_PUBLIC_IP_HELPER_TEXT, + NOT_NATTED_HELPER_TEXT, +} from 'src/features/VPCs/constants'; import { rest, server } from 'src/mocks/testServer'; import { queryClientFactory } from 'src/queries/base'; import { mockMatchMedia, renderWithTheme } from 'src/utilities/testHelpers'; -import { padList } from './LinodeConfigDialog'; -import { LinodeConfigDialog } from './LinodeConfigDialog'; +import { + LinodeConfigDialog, + unrecommendedConfigNoticeSelector, +} from './LinodeConfigDialog'; +import { MemoryLimit, padList } from './LinodeConfigDialog'; const queryClient = queryClientFactory(); @@ -68,4 +76,119 @@ describe('LinodeConfigDialog', () => { }); }); }); + + const publicInterface = LinodeConfigInterfaceFactory.build({ + primary: true, + purpose: 'public', + }); + + const vpcInterface = LinodeConfigInterfaceFactory.build({ + ipv4: { + nat_1_1: '10.0.0.0', + }, + primary: false, + purpose: 'vpc', + }); + + const vpcInterfaceWithoutNAT = LinodeConfigInterfaceFactory.build({ + primary: false, + purpose: 'vpc', + }); + + const editableFields = { + devices: {}, + helpers: { + devtmpfs_automount: true, + distro: true, + modules_dep: true, + network: true, + updatedb_disabled: true, + }, + initrd: null, + interfaces: [publicInterface, vpcInterface], + label: 'Test', + root_device: '/dev/sda', + setMemoryLimit: 'no_limit' as MemoryLimit, + useCustomRoot: false, + }; + + describe('unrecommendedConfigNoticeSelector function', () => { + it('should return a with NATTED_PUBLIC_IP_HELPER_TEXT under the appropriate conditions', () => { + const valueReturned = unrecommendedConfigNoticeSelector({ + _interface: vpcInterface, + primaryInterfaceIndex: editableFields.interfaces.findIndex( + (element) => element.primary === true + ), + thisIndex: editableFields.interfaces.findIndex( + (element) => element.purpose === 'vpc' + ), + values: editableFields, + }); + + expect(valueReturned?.props.text).toEqual(NATTED_PUBLIC_IP_HELPER_TEXT); + }); + + it('should return a with LINODE_UNREACHABLE_HELPER_TEXT under the appropriate conditions', () => { + const editableFieldsWithVPCInterfaceNotNatted = { + ...editableFields, + interfaces: [publicInterface, vpcInterfaceWithoutNAT], + }; + + const valueReturned = unrecommendedConfigNoticeSelector({ + _interface: vpcInterfaceWithoutNAT, + primaryInterfaceIndex: editableFields.interfaces.findIndex( + (element) => element.primary === true + ), + thisIndex: editableFields.interfaces.findIndex( + (element) => element.purpose === 'vpc' + ), + values: editableFieldsWithVPCInterfaceNotNatted, + }); + + expect(valueReturned?.props.text).toEqual(LINODE_UNREACHABLE_HELPER_TEXT); + }); + + it('should return a with NOT_NATTED_HELPER_TEXT under the appropriate conditions', () => { + const vpcInterfacePrimaryWithoutNAT = { + ...vpcInterfaceWithoutNAT, + primary: true, + }; + + const editableFieldsWithSingleInterface = { + ...editableFields, + interfaces: [vpcInterfacePrimaryWithoutNAT], + }; + + const valueReturned = unrecommendedConfigNoticeSelector({ + _interface: vpcInterfacePrimaryWithoutNAT, + primaryInterfaceIndex: editableFieldsWithSingleInterface.interfaces.findIndex( + (element) => element.primary === true + ), + thisIndex: editableFieldsWithSingleInterface.interfaces.findIndex( + (element) => element.purpose === 'vpc' + ), + values: editableFieldsWithSingleInterface, + }); + + expect(valueReturned?.props.text).toEqual(NOT_NATTED_HELPER_TEXT); + }); + + it('should not return a outside of the prescribed conditions', () => { + const editableFieldsWithoutVPCInterface = { + ...editableFields, + interfaces: [publicInterface], + }; + + const valueReturned = unrecommendedConfigNoticeSelector({ + _interface: publicInterface, + primaryInterfaceIndex: editableFieldsWithoutVPCInterface.interfaces.findIndex( + (element) => element.primary === true + ), + thisIndex: 0, + values: editableFieldsWithoutVPCInterface, + }); + + expect(valueReturned?.props.text).toBe(undefined); + }); + }); }); diff --git a/packages/manager/src/features/Linodes/LinodesDetail/LinodeConfigs/LinodeConfigDialog.tsx b/packages/manager/src/features/Linodes/LinodesDetail/LinodeConfigs/LinodeConfigDialog.tsx index b14a87507b6..5b353e408ea 100644 --- a/packages/manager/src/features/Linodes/LinodesDetail/LinodeConfigs/LinodeConfigDialog.tsx +++ b/packages/manager/src/features/Linodes/LinodesDetail/LinodeConfigs/LinodeConfigDialog.tsx @@ -35,6 +35,11 @@ import { TooltipIcon } from 'src/components/TooltipIcon'; import { Typography } from 'src/components/Typography'; import { DeviceSelection } from 'src/features/Linodes/LinodesDetail/LinodeRescue/DeviceSelection'; import { titlecase } from 'src/features/Linodes/presentation'; +import { + LINODE_UNREACHABLE_HELPER_TEXT, + 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 { @@ -79,7 +84,7 @@ interface Helpers { type RunLevel = 'binbash' | 'default' | 'single'; type VirtMode = 'fullvirt' | 'paravirt'; -type MemoryLimit = 'no_limit' | 'set_limit'; +export type MemoryLimit = 'no_limit' | 'set_limit'; interface EditableFields { comments?: string; @@ -986,36 +991,46 @@ export const LinodeConfigDialog = (props: Props) => { )} {values.interfaces.map((thisInterface, idx) => { return ( - - handleInterfaceChange(idx, newInterface) - } - ipamAddress={thisInterface.ipam_address} - key={`eth${idx}-interface`} - label={thisInterface.label} - nattedIPv4Address={thisInterface.ipv4?.nat_1_1} - purpose={thisInterface.purpose} - readOnly={isReadOnly} - region={linode?.region} - regionHasVLANs={regionHasVLANS} - regionHasVPCs={regionHasVPCs} - slotNumber={idx} - subnetId={thisInterface.subnet_id} - vpcIPv4={thisInterface.ipv4?.vpc} - vpcId={thisInterface.vpc_id} - /> + <> + {unrecommendedConfigNoticeSelector({ + _interface: thisInterface, + primaryInterfaceIndex, + thisIndex: idx, + values, + })} + + handleInterfaceChange(idx, newInterface) + } + ipamAddress={thisInterface.ipam_address} + key={`eth${idx}-interface`} + label={thisInterface.label} + nattedIPv4Address={thisInterface.ipv4?.nat_1_1} + purpose={thisInterface.purpose} + readOnly={isReadOnly} + region={linode?.region} + regionHasVLANs={regionHasVLANS} + regionHasVPCs={regionHasVPCs} + slotNumber={idx} + subnetId={thisInterface.subnet_id} + vpcIPv4={thisInterface.ipv4?.vpc} + vpcId={thisInterface.vpc_id} + /> + ); })} @@ -1181,3 +1196,74 @@ const isUsingCustomRoot = (value: string) => '/dev/sdg', '/dev/sdh', ].includes(value) === false; + +const noticeForScenario = (scenarioText: string) => ( + +); + +/** + * + * @param _interface the current config interface being passed in + * @param primaryInterfaceIndex the index of the primary interface + * @param thisIndex the index of the current config interface within the `interfaces` array of the `config` object + * @param values the values held in Formik state, having a type of `EditableFields` + * @returns JSX.Element | null + */ +export const unrecommendedConfigNoticeSelector = ({ + _interface, + primaryInterfaceIndex, + thisIndex, + values, +}: { + _interface: ExtendedInterface; + primaryInterfaceIndex: number | undefined; + thisIndex: number; + values: EditableFields; +}): JSX.Element | null => { + const vpcInterface = _interface.purpose === 'vpc'; + const nattedIPv4Address = Boolean(_interface.ipv4?.nat_1_1); + + const filteredInterfaces = values.interfaces.filter( + (_interface) => _interface.purpose !== 'none' + ); + + // Edge case: users w/ ability to have multiple VPC interfaces. Scenario 1 & 2 notices not helpful if that's done + const primaryInterfaceIsVPC = + primaryInterfaceIndex !== undefined + ? values.interfaces[primaryInterfaceIndex].purpose === 'vpc' + : false; + + /* + Scenario 1: + - the interface passed in to this function is a VPC interface + - the index of the primary interface !== the index of the interface passed in to this function + - nattedIPv4Address (i.e., "Assign a public IPv4 address for this Linode" checked) + + Scenario 2: + - all of Scenario 1, except: !nattedIPv4Address (i.e., "Assign a public IPv4 address for this Linode" unchecked) + + Scenario 3: + - only eth0 populated, and it is a VPC interface + + If not one of the above scenarios, do not display a warning notice re: configuration + */ + if ( + vpcInterface && + primaryInterfaceIndex !== thisIndex && + !primaryInterfaceIsVPC + ) { + return nattedIPv4Address + ? noticeForScenario(NATTED_PUBLIC_IP_HELPER_TEXT) + : noticeForScenario(LINODE_UNREACHABLE_HELPER_TEXT); + } + + if (filteredInterfaces.length === 1 && vpcInterface && !nattedIPv4Address) { + return noticeForScenario(NOT_NATTED_HELPER_TEXT); + } + + return null; +}; diff --git a/packages/manager/src/features/Linodes/LinodesDetail/LinodeSettings/InterfaceSelect.tsx b/packages/manager/src/features/Linodes/LinodesDetail/LinodeSettings/InterfaceSelect.tsx index 0ca6633a984..6a299c39682 100644 --- a/packages/manager/src/features/Linodes/LinodesDetail/LinodeSettings/InterfaceSelect.tsx +++ b/packages/manager/src/features/Linodes/LinodesDetail/LinodeSettings/InterfaceSelect.tsx @@ -11,15 +11,16 @@ import { Divider } from 'src/components/Divider'; import Select, { Item } from 'src/components/EnhancedSelect/Select'; import { Stack } from 'src/components/Stack'; import { TextField } from 'src/components/TextField'; +import { Typography } from 'src/components/Typography'; import { VPCPanel } from 'src/features/Linodes/LinodesCreate/VPCPanel'; import { useFlags } from 'src/hooks/useFlags'; 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 { Typography } from 'src/components/Typography'; interface Props { + errors: VPCInterfaceErrors & OtherInterfaceErrors; fromAddonsPanel?: boolean; handleChange: (updatedInterface: ExtendedInterface) => void; ipamAddress?: null | string; @@ -27,13 +28,11 @@ interface Props { purpose: ExtendedPurpose; readOnly: boolean; region?: string; - slotNumber: number; regionHasVLANs?: boolean; regionHasVPCs?: boolean; + slotNumber: number; } - -interface VPCStateErrors { - ipamError?: string; +interface VPCInterfaceErrors { labelError?: string; publicIPv4Error?: string; subnetError?: string; @@ -41,8 +40,12 @@ interface VPCStateErrors { vpcIPv4Error?: string; } +interface OtherInterfaceErrors { + ipamError?: string; + primaryError?: string; +} + interface VPCState { - errors: VPCStateErrors; nattedIPv4Address?: string; subnetId?: null | number; vpcIPv4?: string; @@ -69,12 +72,12 @@ export const InterfaceSelect = (props: CombinedProps) => { purpose, readOnly, region, + regionHasVLANs, + regionHasVPCs, slotNumber, subnetId, vpcIPv4, vpcId, - regionHasVLANs, - regionHasVPCs, } = props; const theme = useTheme(); diff --git a/packages/manager/src/features/VPCs/constants.ts b/packages/manager/src/features/VPCs/constants.ts index 28975d34341..bf1fdc9ab9f 100644 --- a/packages/manager/src/features/VPCs/constants.ts +++ b/packages/manager/src/features/VPCs/constants.ts @@ -30,3 +30,13 @@ export const VPC_FEEDBACK_FORM_URL = export const VPC_REBOOT_MESSAGE = 'The VPC configuration has been updated and the Linode needs to be rebooted.'; + +// Linode Config dialog helper text for unrecommended configurations +export const LINODE_UNREACHABLE_HELPER_TEXT = + 'This network configuration is not recommended. The Linode will not be reachable or be able to reach Linodes in the other subnets of the VPC. We recommend selecting VPC as the primary interface and checking the “Assign a public IPv4 address for this Linode” checkbox.'; + +export const NATTED_PUBLIC_IP_HELPER_TEXT = + "This network configuration is not recommended. The VPC interface with 1:1 NAT will use the Public IP even if it's not on the default network interface. The Linode will lose access to public network connectivity since the default route isn't able to route through the public IPv4 address. We recommend selecting VPC as the primary interface."; + +export const NOT_NATTED_HELPER_TEXT = + 'The Linode will not be able to access the internet. If this Linode needs access to the internet we recommend checking the “Assign a public IPv4 address for this Linode” checkbox to enable 1:1 NAT on the VPC interface.';