From 137516422dbf61583acd1772525bb557fbd9655e Mon Sep 17 00:00:00 2001 From: Nick De Villiers Date: Tue, 20 Aug 2024 11:40:02 +0100 Subject: [PATCH] feat(machines): Add new IP address field to Edit Physical form MAASENG-3453 (#5511) - Added `PrefixedIpInput` to the "Edit Physical" form - Added subnet-based IP validation to the form schema Resolves [MAASENG-3453](https://warthogs.atlassian.net/browse/MAASENG-3453) --- .../EditPhysicalForm.test.tsx | 71 +++++++++++- .../EditPhysicalForm/EditPhysicalForm.tsx | 102 ++++++++++++++++-- .../NetworkFields/NetworkFields.tsx | 84 +++++++++++++-- 3 files changed, 243 insertions(+), 14 deletions(-) diff --git a/src/app/machines/views/MachineDetails/MachineNetwork/EditPhysicalForm/EditPhysicalForm.test.tsx b/src/app/machines/views/MachineDetails/MachineNetwork/EditPhysicalForm/EditPhysicalForm.test.tsx index ef35d37adf..b6f65c8d33 100644 --- a/src/app/machines/views/MachineDetails/MachineNetwork/EditPhysicalForm/EditPhysicalForm.test.tsx +++ b/src/app/machines/views/MachineDetails/MachineNetwork/EditPhysicalForm/EditPhysicalForm.test.tsx @@ -35,7 +35,14 @@ describe("EditPhysicalForm", () => { }), }), subnet: factory.subnetState({ - items: [factory.subnet({ id: 1, vlan: 1 }), factory.subnet()], + items: [ + factory.subnet({ + id: 1, + vlan: 1, + cidr: "10.0.0.0/24", + statistics: factory.subnetStatistics({ ip_version: 4 }), + }), + ], loaded: true, }), vlan: factory.vlanState({ @@ -69,6 +76,68 @@ describe("EditPhysicalForm", () => { expect(screen.getByText(/loading/i)).toBeInTheDocument(); }); + it("displays an error if an IP address is not valid", async () => { + renderWithBrowserRouter( + , + { route: "/machines", state } + ); + + await userEvent.selectOptions( + screen.getByRole("combobox", { name: "Subnet" }), + state.subnet.items[0].id.toString() + ); + + await userEvent.selectOptions( + screen.getByRole("combobox", { name: "IP assignment" }), + "Static (Client configured)" + ); + + await userEvent.type( + screen.getByRole("textbox", { name: "IP address" }), + "abc" + ); + + await userEvent.click( + screen.getByRole("button", { name: "Save interface" }) + ); + + expect( + screen.getByText("This is not a valid IP address") + ).toBeInTheDocument(); + }); + + it("displays an error if an IP address is out of range", async () => { + renderWithBrowserRouter( + , + { route: "/machines", state } + ); + + await userEvent.selectOptions( + screen.getByRole("combobox", { name: "Subnet" }), + state.subnet.items[0].id.toString() + ); + + await userEvent.selectOptions( + screen.getByRole("combobox", { name: "IP assignment" }), + "Static (Client configured)" + ); + + await userEvent.clear(screen.getByRole("textbox", { name: "IP address" })); + + await userEvent.type( + screen.getByRole("textbox", { name: "IP address" }), + "255" + ); + + await userEvent.click( + screen.getByRole("button", { name: "Save interface" }) + ); + + expect( + screen.getByText("The IP address is outside of the subnet's range.") + ).toBeInTheDocument(); + }); + it("correctly dispatches actions to edit a physical interface", async () => { const store = mockStore(state); renderWithBrowserRouter( diff --git a/src/app/machines/views/MachineDetails/MachineNetwork/EditPhysicalForm/EditPhysicalForm.tsx b/src/app/machines/views/MachineDetails/MachineNetwork/EditPhysicalForm/EditPhysicalForm.tsx index 9ae329174a..14ba7c1d27 100644 --- a/src/app/machines/views/MachineDetails/MachineNetwork/EditPhysicalForm/EditPhysicalForm.tsx +++ b/src/app/machines/views/MachineDetails/MachineNetwork/EditPhysicalForm/EditPhysicalForm.tsx @@ -1,6 +1,8 @@ import { useCallback } from "react"; import { Spinner } from "@canonical/react-components"; +import * as ipaddr from "ipaddr.js"; +import { isIP, isIPv4 } from "is-ip"; import { useDispatch, useSelector } from "react-redux"; import * as Yup from "yup"; @@ -10,6 +12,7 @@ import EditPhysicalFields from "./EditPhysicalFields"; import type { EditPhysicalValues } from "./types"; import FormikForm from "@/app/base/components/FormikForm"; +import { formatIpAddress } from "@/app/base/components/PrefixedIpInput"; import { useFetchActions, useIsAllNetworkingDisabled } from "@/app/base/hooks"; import { MAC_ADDRESS_REGEX } from "@/app/base/validation"; import { useMachineDetailsForm } from "@/app/machines/hooks"; @@ -23,6 +26,7 @@ import { isMachineDetails } from "@/app/store/machine/utils"; import type { RootState } from "@/app/store/root/types"; import { subnetActions } from "@/app/store/subnet"; import subnetSelectors from "@/app/store/subnet/selectors"; +import { NetworkLinkMode } from "@/app/store/types/enum"; import type { NetworkInterface, NetworkLink, @@ -37,6 +41,11 @@ import { import { vlanActions } from "@/app/store/vlan"; import vlanSelectors from "@/app/store/vlan/selectors"; import { preparePayload } from "@/app/utils"; +import { + getImmutableAndEditableOctets, + getIpRangeFromCidr, + isIpInSubnet, +} from "@/app/utils/subnetIpRange"; type Props = { close: () => void; @@ -54,6 +63,52 @@ const InterfaceSchema = Yup.object().shape({ .required("MAC address is required"), name: Yup.string(), tags: Yup.array().of(Yup.string()), + ip_address: Yup.string().when("mode", { + is: NetworkLinkMode.STATIC, + then: Yup.string() + .test({ + name: "ip-is-valid", + message: "This is not a valid IP address", + test: (ip_address, context) => { + // Wrap this in a try/catch since the subnet might not be loaded yet + try { + return isIP( + formatIpAddress(ip_address, context.parent.subnet_cidr as string) + ); + } catch (e) { + return false; + } + }, + }) + .test({ + name: "ip-is-in-subnet", + message: "The IP address is outside of the subnet's range.", + test: (ip_address, context) => { + // Wrap this in a try/catch since the subnet might not be loaded yet + try { + const cidr: string = context.parent.subnet_cidr; + const networkAddress = cidr.split("/")[0]; + const prefixLength = parseInt(cidr.split("/")[1]); + const subnetIsIpv4 = isIPv4(networkAddress); + + const ip = formatIpAddress(ip_address, cidr); + if (subnetIsIpv4) { + return isIpInSubnet(ip, cidr); + } else { + try { + const addr = ipaddr.parse(ip); + const netAddr = ipaddr.parse(networkAddress); + return addr.match(netAddr, prefixLength); + } catch (e) { + return false; + } + } + } catch (e) { + return false; + } + }, + }), + }), }); const EditPhysicalForm = ({ @@ -106,6 +161,30 @@ const EditPhysicalForm = ({ ); const ipAddress = getInterfaceIPAddress(machine, fabrics, vlans, nic, link); + const getInitialIpAddressValue = () => { + if (subnet && ipAddress) { + const [startIp, endIp] = getIpRangeFromCidr(subnet.cidr); + const [immutableOctets, _] = getImmutableAndEditableOctets( + startIp, + endIp + ); + const networkAddress = subnet.cidr.split("/")[0]; + const ipv6Prefix = networkAddress.substring( + 0, + networkAddress.lastIndexOf(":") + ); + const subnetIsIpv4 = isIPv4(networkAddress); + + if (subnetIsIpv4) { + return ipAddress.replace(`${immutableOctets}.`, ""); + } else { + return ipAddress.replace(`${ipv6Prefix}`, ""); + } + } else { + return ""; + } + }; + return ( aria-label="Edit physical" @@ -117,7 +196,7 @@ const EditPhysicalForm = ({ interface_speed: isNaN(Number(nic.interface_speed)) ? 0 : nic.interface_speed / 1000, - ip_address: ipAddress || "", + ip_address: getInitialIpAddressValue(), // The current link is required to update the subnet and ip address. link_id: linkId || "", link_speed: isNaN(Number(nic.link_speed)) ? 0 : nic.link_speed / 1000, @@ -125,6 +204,7 @@ const EditPhysicalForm = ({ mode: getLinkMode(link), name: nic.name, subnet: subnet?.id || "", + subnet_cidr: subnet?.cidr || "", tags: nic.tags, vlan: nic.vlan_id, }} @@ -137,15 +217,25 @@ const EditPhysicalForm = ({ onSubmit={(values) => { // Clear the errors from the previous submission. dispatch(cleanup()); + + const ip = + values.ip_address && values.subnet_cidr + ? formatIpAddress(values.ip_address, values.subnet_cidr) + : ""; type Payload = EditPhysicalValues & { interface_id: NetworkInterface["id"]; system_id: Machine["system_id"]; }; - const payload: Payload = preparePayload({ - ...values, - interface_id: nic.id, - system_id: systemId, - }); + const payload: Payload = preparePayload( + { + ...values, + interface_id: nic.id, + system_id: systemId, + ip_address: ip, + }, + [], + ["subnet_cidr"] + ); // Convert the speeds back from GB. if (!isNaN(Number(payload.link_speed))) { payload.link_speed = Number(payload.link_speed) * 1000; diff --git a/src/app/machines/views/MachineDetails/MachineNetwork/NetworkFields/NetworkFields.tsx b/src/app/machines/views/MachineDetails/MachineNetwork/NetworkFields/NetworkFields.tsx index 1cbbc3a321..61eb7d4e9c 100644 --- a/src/app/machines/views/MachineDetails/MachineNetwork/NetworkFields/NetworkFields.tsx +++ b/src/app/machines/views/MachineDetails/MachineNetwork/NetworkFields/NetworkFields.tsx @@ -1,18 +1,21 @@ +import { useEffect } from "react"; + import { useFormikContext } from "formik"; +import { isIPv4 } from "is-ip"; import { useSelector } from "react-redux"; import * as Yup from "yup"; import FabricSelect from "@/app/base/components/FabricSelect"; import FormikField from "@/app/base/components/FormikField"; import LinkModeSelect from "@/app/base/components/LinkModeSelect"; +import PrefixedIpInput from "@/app/base/components/PrefixedIpInput"; import SubnetSelect from "@/app/base/components/SubnetSelect"; import VLANSelect from "@/app/base/components/VLANSelect"; import fabricSelectors from "@/app/store/fabric/selectors"; import type { Fabric } from "@/app/store/fabric/types"; import subnetSelectors from "@/app/store/subnet/selectors"; import type { Subnet } from "@/app/store/subnet/types"; -import { NetworkLinkMode } from "@/app/store/types/enum"; -import type { NetworkInterfaceTypes } from "@/app/store/types/enum"; +import { NetworkInterfaceTypes, NetworkLinkMode } from "@/app/store/types/enum"; import type { NetworkInterface, NetworkLink, @@ -20,12 +23,17 @@ import type { } from "@/app/store/types/node"; import type { VLAN } from "@/app/store/vlan/types"; import { toFormikNumber } from "@/app/utils"; +import { + getImmutableAndEditableOctets, + getIpRangeFromCidr, +} from "@/app/utils/subnetIpRange"; export type NetworkValues = { ip_address?: NetworkLink["ip_address"]; mode?: NetworkLinkMode | ""; fabric: NodeVlan["fabric_id"] | ""; subnet?: NetworkLink["subnet_id"] | ""; + subnet_cidr?: Subnet["cidr"] | ""; vlan: NetworkInterface["vlan_id"] | ""; }; @@ -42,6 +50,7 @@ export const networkFieldsInitialValues = { mode: "", fabric: "", subnet: "", + subnet_cidr: "", vlan: "", } as NetworkValues; @@ -74,6 +83,22 @@ const NetworkFields = ({ const subnets: Subnet[] = useSelector(subnetSelectors.all); const { handleChange, setFieldValue, values } = useFormikContext(); + + useEffect(() => { + if ( + interfaceType === NetworkInterfaceTypes.PHYSICAL && + subnets && + values.subnet + ) { + const subnet = subnets.find( + ({ id }) => id === toFormikNumber(values.subnet) + ); + if (subnet) { + setFieldValue("subnet_cidr", subnet.cidr); + } + } + }, [interfaceType, setFieldValue, subnets, values.subnet]); + const resetFollowingFields = ( name: keyof NetworkValues, hasSubnet?: boolean @@ -92,6 +117,7 @@ const NetworkFields = ({ setFieldValue(fieldOrder[i], value); } }; + return ( <> id === toFormikNumber(values.subnet) ); - setFieldValue( - "ip_address", - subnet?.statistics.first_address || "" - ); + if ( + interfaceType === NetworkInterfaceTypes.PHYSICAL && + subnet && + editing + ) { + const [startIp, endIp] = getIpRangeFromCidr(subnet.cidr); + const [immutableOctets, _] = getImmutableAndEditableOctets( + startIp, + endIp + ); + const networkAddress = subnet.cidr.split("/")[0]; + const ipv6Prefix = networkAddress.substring( + 0, + networkAddress.lastIndexOf(":") + ); + const subnetIsIpv4 = isIPv4(networkAddress); + + if (subnetIsIpv4) { + setFieldValue( + "ip_address", + subnet.statistics.first_address.replace( + `${immutableOctets}.`, + "" + ) + ); + } else { + setFieldValue( + "ip_address", + subnet.statistics.first_address.replace(`${ipv6Prefix}`, "") + ); + } + } else { + setFieldValue( + "ip_address", + subnet?.statistics.first_address || "" + ); + } } else { setFieldValue("ip_address", ""); } @@ -167,7 +226,18 @@ const NetworkFields = ({ /> ) : null} {values.mode === NetworkLinkMode.STATIC ? ( - + interfaceType === NetworkInterfaceTypes.PHYSICAL && editing ? ( + values.subnet_cidr ? ( + + ) : null + ) : ( + + ) ) : null} );