Skip to content

Commit

Permalink
fix(machines): Allow ports at the end of IPMI addresses LP#2087965 (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ndv99 authored Dec 10, 2024
1 parent 0245ba2 commit 847d318
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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,
}),
Expand All @@ -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(),
}),
}),
});
Expand Down Expand Up @@ -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(<PowerForm systemId="def456" />, { 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(<PowerForm systemId="def456" />, { 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"],
Expand Down
41 changes: 37 additions & 4 deletions src/app/store/general/utils/powerTypes.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions src/app/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ export {
timeSpanToMinutes,
} from "./timeSpan";
export { parseCommaSeparatedValues } from "./parseCommaSeparatedValues";
export { isValidPortNumber } from "./isValidPortNumber";
15 changes: 15 additions & 0 deletions src/app/utils/isValidPortNumber.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
9 changes: 9 additions & 0 deletions src/app/utils/isValidPortNumber.ts
Original file line number Diff line number Diff line change
@@ -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;
};

0 comments on commit 847d318

Please sign in to comment.