From 847d318e187cc443e7277a062b47e415b8c16b4b Mon Sep 17 00:00:00 2001 From: Nick De Villiers Date: Tue, 10 Dec 2024 16:25:55 +0000 Subject: [PATCH] fix(machines): Allow ports at the end of IPMI addresses LP#2087965 (#5560) Resolves [LP#2087965](https://bugs.launchpad.net/maas/+bug/2087965) --- .../PowerForm/PowerForm.test.tsx | 102 +++++++++++++++++- src/app/store/general/utils/powerTypes.ts | 41 ++++++- src/app/utils/index.ts | 1 + src/app/utils/isValidPortNumber.test.ts | 15 +++ src/app/utils/isValidPortNumber.ts | 9 ++ 5 files changed, 163 insertions(+), 5 deletions(-) create mode 100644 src/app/utils/isValidPortNumber.test.ts create mode 100644 src/app/utils/isValidPortNumber.ts diff --git a/src/app/machines/views/MachineDetails/MachineConfiguration/PowerForm/PowerForm.test.tsx b/src/app/machines/views/MachineDetails/MachineConfiguration/PowerForm/PowerForm.test.tsx index 534a0f7999..0a83c93470 100644 --- a/src/app/machines/views/MachineDetails/MachineConfiguration/PowerForm/PowerForm.test.tsx +++ b/src/app/machines/views/MachineDetails/MachineConfiguration/PowerForm/PowerForm.test.tsx @@ -10,7 +10,13 @@ import { PowerFieldScope, PowerFieldType } from "@/app/store/general/types"; import { machineActions } from "@/app/store/machine"; import type { RootState } from "@/app/store/root/types"; import * as factory from "@/testing/factories"; -import { userEvent, render, screen, waitFor } from "@/testing/utils"; +import { + userEvent, + render, + screen, + waitFor, + renderWithBrowserRouter, +} from "@/testing/utils"; const mockStore = configureStore(); @@ -42,6 +48,17 @@ beforeEach(() => { ], name: PowerTypeNames.APC, }), + factory.powerType({ + fields: [ + factory.powerField({ + name: "ip_address", + label: "IP address", + field_type: PowerFieldType.IP_ADDRESS, + scope: PowerFieldScope.NODE, + }), + ], + name: PowerTypeNames.IPMI, + }), ], loaded: true, }), @@ -53,9 +70,15 @@ beforeEach(() => { power_type: PowerTypeNames.AMT, system_id: "abc123", }), + factory.machineDetails({ + permissions: ["edit"], + power_type: PowerTypeNames.IPMI, + system_id: "def456", + }), ], statuses: factory.machineStatuses({ abc123: factory.machineStatus(), + def456: factory.machineStatus(), }), }), }); @@ -116,6 +139,83 @@ it("renders read-only text fields until edit button is pressed", async () => { ).toBeInTheDocument(); }); +it("can validate IPv6 addresses with a port for IPMI power type", async () => { + const store = mockStore(state); + renderWithBrowserRouter(, { store }); + + await userEvent.click( + screen.getAllByRole("button", { name: Labels.EditButton })[0] + ); + + await userEvent.selectOptions( + screen.getByRole("combobox", { name: "Power type" }), + PowerTypeNames.IPMI + ); + + await userEvent.clear(screen.getByRole("textbox", { name: "IP address" })); + await userEvent.type( + screen.getByRole("textbox", { name: "IP address" }), + "not an ip address" + ); + + await userEvent.tab(); + + expect( + screen.getByText("Please enter a valid IP address.") + ).toBeInTheDocument(); + + await userEvent.clear(screen.getByRole("textbox", { name: "IP address" })); + await userEvent.type( + screen.getByRole("textbox", { name: "IP address" }), + // This is entered as [2001:db8::1]:8080, since square brackets are + // special characters in testing-library user events and can be escaped by doubling. + "[[2001:db8::1]:8080" + ); + + await userEvent.tab(); + + expect( + screen.queryByText("Please enter a valid IP address.") + ).not.toBeInTheDocument(); +}); + +it("can validate IPv4 addresses with a port for IPMI power type", async () => { + const store = mockStore(state); + renderWithBrowserRouter(, { store }); + + await userEvent.click( + screen.getAllByRole("button", { name: Labels.EditButton })[0] + ); + + await userEvent.selectOptions( + screen.getByRole("combobox", { name: "Power type" }), + PowerTypeNames.IPMI + ); + + await userEvent.clear(screen.getByRole("textbox", { name: "IP address" })); + await userEvent.type( + screen.getByRole("textbox", { name: "IP address" }), + "not an ip address" + ); + + await userEvent.tab(); + + expect( + screen.getByText("Please enter a valid IP address.") + ).toBeInTheDocument(); + + await userEvent.clear(screen.getByRole("textbox", { name: "IP address" })); + await userEvent.type( + screen.getByRole("textbox", { name: "IP address" }), + "192.168.0.2:8080" + ); + + await userEvent.tab(); + + expect( + screen.queryByText("Please enter a valid IP address.") + ).not.toBeInTheDocument(); +}); it("correctly dispatches an action to update a machine's power", async () => { const machine = factory.machineDetails({ permissions: ["edit"], diff --git a/src/app/store/general/utils/powerTypes.ts b/src/app/store/general/utils/powerTypes.ts index 06409386cd..4fa51ac28c 100644 --- a/src/app/store/general/utils/powerTypes.ts +++ b/src/app/store/general/utils/powerTypes.ts @@ -1,10 +1,11 @@ -import { isIP } from "is-ip"; +import { isIP, isIPv6 } from "is-ip"; import * as Yup from "yup"; import type { ObjectShape } from "yup/lib/object"; import type { PowerField, PowerType } from "@/app/store/general/types"; import { PowerFieldScope, PowerFieldType } from "@/app/store/general/types"; import type { PowerParameters } from "@/app/store/types/node"; +import { isValidPortNumber } from "@/app/utils/isValidPortNumber"; /** * Formats power parameters by what is expected by the api. Also, React expects @@ -56,12 +57,44 @@ const getPowerFieldSchema = (fieldType: PowerFieldType) => { case PowerFieldType.MULTIPLE_CHOICE: return Yup.array().of(Yup.string()); case PowerFieldType.IP_ADDRESS: - case PowerFieldType.VIRSH_ADDRESS: - case PowerFieldType.LXD_ADDRESS: return Yup.string().test({ name: "is-ip-address", message: "Please enter a valid IP address.", - test: (value) => isIP(value as string), + test: (value) => { + if (typeof value !== "string") { + return false; + } + // reject if value contains whitespace + if (value.includes(" ")) { + return false; + } + if (value.includes("[") && value.includes("]")) { + // This is an IPv6 address with a port number + const openingBracketIndex = value.indexOf("["); + const closingBracketIndex = value.indexOf("]"); + const ip = value.slice( + openingBracketIndex + 1, + closingBracketIndex + ); + // We use +2 here to include the `:` before the port number + const port = value.slice(closingBracketIndex + 2); + return ( + isIPv6(ip) && + !isNaN(parseInt(port)) && + isValidPortNumber(parseInt(port)) + ); + } else if (value.split(":").length === 2) { + // This is an IPv4 address with a port number + const [ip, port] = value.split(":"); + return ( + isIP(ip) && + !isNaN(parseInt(port)) && + isValidPortNumber(parseInt(port)) + ); + } else { + return isIP(value); + } + }, }); default: return Yup.string(); diff --git a/src/app/utils/index.ts b/src/app/utils/index.ts index f40bb889b9..a65da56276 100644 --- a/src/app/utils/index.ts +++ b/src/app/utils/index.ts @@ -36,3 +36,4 @@ export { timeSpanToMinutes, } from "./timeSpan"; export { parseCommaSeparatedValues } from "./parseCommaSeparatedValues"; +export { isValidPortNumber } from "./isValidPortNumber"; diff --git a/src/app/utils/isValidPortNumber.test.ts b/src/app/utils/isValidPortNumber.test.ts new file mode 100644 index 0000000000..c311d856b8 --- /dev/null +++ b/src/app/utils/isValidPortNumber.test.ts @@ -0,0 +1,15 @@ +import { isValidPortNumber } from "."; + +it("returns true for any number between 0 and 65535", () => { + for (let i = 0; i <= 65535; i++) { + expect(isValidPortNumber(i)).toBe(true); + } +}); + +it("returns false for numbers larger than 65535", () => { + expect(isValidPortNumber(65536)).toBe(false); +}); + +it("returns false for numbers smaller than 0", () => { + expect(isValidPortNumber(-1)).toBe(false); +}); diff --git a/src/app/utils/isValidPortNumber.ts b/src/app/utils/isValidPortNumber.ts new file mode 100644 index 0000000000..1a8ac73098 --- /dev/null +++ b/src/app/utils/isValidPortNumber.ts @@ -0,0 +1,9 @@ +/** + * Checks if a given port number is valid (between 0 and 65535). + * + * @param port The port number to check. + * @returns True if valid, false otherwise. + */ +export const isValidPortNumber = (port: number): boolean => { + return port >= 0 && port <= 65535; +};