Skip to content

Commit

Permalink
feat(devices): Integrate dynamic IP validation with "Add Device" form…
Browse files Browse the repository at this point in the history
… MAASENG-2982 (#5481)
  • Loading branch information
ndv99 authored Jun 26, 2024
1 parent e665732 commit 8c17eb1
Show file tree
Hide file tree
Showing 9 changed files with 267 additions and 97 deletions.
1 change: 1 addition & 0 deletions src/app/base/components/PrefixedIpInput/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export { default } from "./PrefixedIpInput";
export { formatIpAddress } from "./utils";
15 changes: 15 additions & 0 deletions src/app/base/components/PrefixedIpInput/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { formatIpAddress } from "./utils";

it("can format an IPv4 address", () => {
const ip = "10";
const cidr = "192.168.0.0/24";

expect(formatIpAddress(ip, cidr)).toBe("192.168.0.10");
});

it("can format an IPv6 address", () => {
const ip = ":10";
const cidr = "2001:db8::/32";

expect(formatIpAddress(ip, cidr)).toBe("2001:db8::10");
});
26 changes: 26 additions & 0 deletions src/app/base/components/PrefixedIpInput/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { isIPv4 } from "is-ip";

import {
getImmutableAndEditableOctets,
getIpRangeFromCidr,
} from "@/app/utils/subnetIpRange";

/**
* Formats the PrefixedIpInput value into a complete IP address.
*
* @param ip The partial IP address to format
* @param cidr The subnet's CIDR notation
* @returns The formatted IP address
*/
export const formatIpAddress = (ip: string | undefined, cidr: string) => {
const [startIp, endIp] = getIpRangeFromCidr(cidr);
const [immutableOctets, _] = getImmutableAndEditableOctets(startIp, endIp);
const networkAddress = cidr.split("/")[0];
const ipv6Prefix = networkAddress.substring(
0,
networkAddress.lastIndexOf(":")
);
const subnetIsIpv4 = isIPv4(networkAddress);

return subnetIsIpv4 ? `${immutableOctets}.${ip}` : `${ipv6Prefix}${ip}`;
};
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ describe("AddDeviceForm", () => {
loaded: true,
}),
subnet: factory.subnetState({
items: [factory.subnet({ id: 0, name: "subnet" })],
items: [
factory.subnet({ id: 0, name: "subnet", cidr: "192.168.1.0/24" }),
],
loaded: true,
}),
zone: factory.zoneState({
Expand Down Expand Up @@ -121,10 +123,7 @@ describe("AddDeviceForm", () => {
"0"
);

await userEvent.type(
getFieldFromCard(0, "IP address", "textbox"),
"192.168.1.1"
);
await userEvent.type(getFieldFromCard(0, "IP address", "textbox"), "1");

await userEvent.type(
getFieldFromCard(1, "MAC address", "textbox"),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { useState } from "react";

import { Col, Row, Spinner, Strip } from "@canonical/react-components";
import ipaddr from "ipaddr.js";
import { isIP, isIPv4 } from "is-ip";
import { useDispatch, useSelector } from "react-redux";
import * as Yup from "yup";

Expand All @@ -10,6 +12,7 @@ import type { AddDeviceValues } from "./types";
import DomainSelect from "@/app/base/components/DomainSelect";
import FormikField from "@/app/base/components/FormikField";
import FormikForm from "@/app/base/components/FormikForm";
import { formatIpAddress } from "@/app/base/components/PrefixedIpInput";
import ZoneSelect from "@/app/base/components/ZoneSelect";
import { useFetchActions, useAddMessage } from "@/app/base/hooks";
import type { ClearSidePanelContent } from "@/app/base/types";
Expand All @@ -23,34 +26,96 @@ import { subnetActions } from "@/app/store/subnet";
import subnetSelectors from "@/app/store/subnet/selectors";
import { zoneActions } from "@/app/store/zone";
import zoneSelectors from "@/app/store/zone/selectors";
import { isIpInSubnet } from "@/app/utils/subnetIpRange";

type Props = {
clearSidePanelContent: ClearSidePanelContent;
};

const AddDeviceInterfaceSchema = Yup.object().shape({
mac: Yup.string()
.matches(MAC_ADDRESS_REGEX, "Invalid MAC address")
.required("MAC address is required"),
ip_assignment: Yup.string().required("IP assignment is required"),
ip_address: Yup.string()
.when("ip_assignment", {
is: (ipAssignment: DeviceIpAssignment) =>
ipAssignment === DeviceIpAssignment.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;
}
},
}),
})
.when("ip_assignment", {
is: (ipAssignment: DeviceIpAssignment) =>
ipAssignment === DeviceIpAssignment.EXTERNAL,
then: Yup.string().test({
name: "ip-is-valid",
message: "This is not a valid IP address",
test: (ip_address) => isIP(`${ip_address}`),
}),
})
.when("ip_assignment", {
is: (ipAssignment: DeviceIpAssignment) =>
ipAssignment === DeviceIpAssignment.STATIC ||
ipAssignment === DeviceIpAssignment.EXTERNAL,
then: Yup.string().required("IP address is required"),
}),
subnet: Yup.number().when("ip_assignment", {
is: (ipAssignment: DeviceIpAssignment) =>
ipAssignment === DeviceIpAssignment.STATIC,
then: Yup.number().required("Subnet is required"),
}),
subnet_cidr: Yup.string(),
});

const AddDeviceSchema = Yup.object().shape({
domain: Yup.string().required("Domain required"),
hostname: hostnameValidation,
interfaces: Yup.array()
.of(
Yup.object().shape({
mac: Yup.string()
.matches(MAC_ADDRESS_REGEX, "Invalid MAC address")
.required("MAC address is required"),
ip_assignment: Yup.string().required("IP assignment is required"),
ip_address: Yup.string().when("ip_assignment", {
is: (ipAssignment: DeviceIpAssignment) =>
ipAssignment === DeviceIpAssignment.STATIC ||
ipAssignment === DeviceIpAssignment.EXTERNAL,
then: Yup.string().required("IP address is required"),
}),
subnet: Yup.number().when("ip_assignment", {
is: (ipAssignment: DeviceIpAssignment) =>
ipAssignment === DeviceIpAssignment.STATIC,
then: Yup.number().required("Subnet is required"),
}),
})
)
.of(AddDeviceInterfaceSchema)
.min(1, "At least one interface must be defined"),
zone: Yup.string().required("Zone required"),
});
Expand Down Expand Up @@ -111,6 +176,8 @@ export const AddDeviceForm = ({
mac: "",
name: "eth0",
subnet: "",
// Capture the subnet CIDR so we can validate the IP address against it.
subnet_cidr: "",
},
],
zone: (zones.length && zones[0].name) || "",
Expand All @@ -125,8 +192,16 @@ export const AddDeviceForm = ({
const { domain, hostname, interfaces, zone } = values;
const normalisedInterfaces = interfaces.map((iface) => {
const subnet = parseInt(iface.subnet);
const ip =
iface.ip_assignment === DeviceIpAssignment.STATIC
? formatIpAddress(iface.ip_address, iface.subnet_cidr)
: iface.ip_assignment === DeviceIpAssignment.EXTERNAL
? iface.ip_address
: null;

// subnet_cidr is omitted, since it's only needed for validation.
return {
ip_address: iface.ip_address || null,
ip_address: ip || null,
ip_assignment: iface.ip_assignment,
mac: iface.mac,
name: iface.name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ describe("AddDeviceInterfaces", () => {
mac: "",
name: "eth0",
subnet: "",
subnet_cidr: "",
},
];
state = factory.rootState({
Expand All @@ -53,7 +54,7 @@ describe("AddDeviceInterfaces", () => {
expect(screen.queryByTestId("ip-address-field")).not.toBeInTheDocument();
});

it("shows the IP address field for external IP assignment", () => {
it("shows the standard IP address field for external IP assignment", () => {
interfaces[0].ip_assignment = DeviceIpAssignment.EXTERNAL;
const store = mockStore(state);
renderWithBrowserRouter(
Expand All @@ -63,11 +64,15 @@ describe("AddDeviceInterfaces", () => {
{ store }
);

expect(screen.queryByTestId("subnet-field")).not.toBeInTheDocument();
expect(screen.getByTestId("ip-address-field")).toBeInTheDocument();

expect(screen.queryByTestId("subnet-field")).not.toBeInTheDocument();
expect(
screen.queryByTestId("prefixed-ip-address-field")
).not.toBeInTheDocument();
});

it("shows both the subnet and IP address fields for static IP assignment", () => {
it("shows the subnet field when static IP assignment is selected", () => {
interfaces[0].ip_assignment = DeviceIpAssignment.STATIC;
const store = mockStore(state);
renderWithBrowserRouter(
Expand All @@ -78,7 +83,25 @@ describe("AddDeviceInterfaces", () => {
);

expect(screen.getByTestId("subnet-field")).toBeInTheDocument();
expect(screen.getByTestId("ip-address-field")).toBeInTheDocument();
expect(
screen.queryByTestId("prefixed-ip-address-field")
).not.toBeInTheDocument();
});

it("shows the prefixed IP address field when a subnet is selected", () => {
interfaces[0].ip_assignment = DeviceIpAssignment.STATIC;
interfaces[0].subnet = state.subnet.items[0].id.toString();

const store = mockStore(state);
renderWithBrowserRouter(
<Formik initialValues={{ interfaces }} onSubmit={vi.fn()}>
<AddDeviceInterfaces />
</Formik>,
{ store }
);

expect(screen.getByTestId("prefixed-ip-address-field")).toBeInTheDocument();
expect(screen.queryByTestId("ip-address-field")).not.toBeInTheDocument();
});

it("can add and remove interfaces", async () => {
Expand Down
Loading

0 comments on commit 8c17eb1

Please sign in to comment.