Skip to content

Commit

Permalink
feat(machines): Add new IP address field to Edit Physical form MAASEN…
Browse files Browse the repository at this point in the history
…G-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)
  • Loading branch information
ndv99 authored Aug 20, 2024
1 parent a3d8316 commit 1375164
Show file tree
Hide file tree
Showing 3 changed files with 243 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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(
<EditPhysicalForm close={vi.fn()} nicId={1} systemId="abc123" />,
{ 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(
<EditPhysicalForm close={vi.fn()} nicId={1} systemId="abc123" />,
{ 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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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";

Expand All @@ -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";
Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -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 = ({
Expand Down Expand Up @@ -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 (
<FormikForm<EditPhysicalValues, MachineEventErrors>
aria-label="Edit physical"
Expand All @@ -117,14 +196,15 @@ 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,
mac_address: nic.mac_address,
mode: getLinkMode(link),
name: nic.name,
subnet: subnet?.id || "",
subnet_cidr: subnet?.cidr || "",
tags: nic.tags,
vlan: nic.vlan_id,
}}
Expand All @@ -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;
Expand Down
Loading

0 comments on commit 1375164

Please sign in to comment.