feat: [M3-7343] – Display conditional Notices depending on config scenarios in Linode Add/Edit Config dialog

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))

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 <Notice /> 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 <Notice /> 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 <Notice /> 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 <Notice /> 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);
+    });
+  });
 });

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 (
-                  <InterfaceSelect
-                    errors={{
-                      ipamError:
-                        formik.errors[`interfaces[${idx}].ipam_address`],
-                      labelError: formik.errors[`interfaces[${idx}].label`],
-                      publicIPv4Error:
-                        formik.errors[`interfaces[${idx}].ipv4.nat_1_1`],
-                      subnetError:
-                        formik.errors[`interfaces[${idx}].subnet_id`],
-                      vpcError: formik.errors[`interfaces[${idx}].vpc_id`],
-                      vpcIPv4Error:
-                        formik.errors[`interfaces[${idx}].ipv4.vpc`],
-                    }}
-                    handleChange={(newInterface: Interface) =>
-                      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,
+                    })}
+                    <InterfaceSelect
+                      errors={{
+                        ipamError:
+                          formik.errors[`interfaces[${idx}].ipam_address`],
+                        labelError: formik.errors[`interfaces[${idx}].label`],
+                        primaryError:
+                          formik.errors[`interfaces[${idx}].primary`],
+                        publicIPv4Error:
+                          formik.errors[`interfaces[${idx}].ipv4.nat_1_1`],
+                        subnetError:
+                          formik.errors[`interfaces[${idx}].subnet_id`],
+                        vpcError: formik.errors[`interfaces[${idx}].vpc_id`],
+                        vpcIPv4Error:
+                          formik.errors[`interfaces[${idx}].ipv4.vpc`],
+                      }}
+                      handleChange={(newInterface: Interface) =>
+                        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}
+                    />
+                  </>
                 );
               })}
             </Grid>
@@ -1181,3 +1196,74 @@ const isUsingCustomRoot = (value: string) =>
     '/dev/sdg',
     '/dev/sdh',
   ].includes(value) === false;
+
+const noticeForScenario = (scenarioText: string) => (
+  <Notice
+    data-testid={'notice-for-unrecommended-scenario'}
+    text={scenarioText}
+    variant="warning"
+  />
+);
+
+/**
+ *
+ * @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;
+};

packages/manager/src/features/Linodes/LinodesDetail/LinodeSettings/InterfaceSelect.tsx

@@ -11,38 +11,41 @@ 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;
   label?: null | string;
   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;
   vpcError?: string;
   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();