diff --git a/ui/src/app/base/hooks.js b/ui/src/app/base/hooks.js index 5f9bacaf86..8fa224810d 100644 --- a/ui/src/app/base/hooks.js +++ b/ui/src/app/base/hooks.js @@ -241,23 +241,31 @@ export const useVisible = (initialValue) => { * @param {Function} generateSchemaFunc - Schema generation function. * @returns {Object} Yup validation schema with power parameters. */ -export const usePowerParametersSchema = (powerType, generateSchemaFunc) => { +export const usePowerParametersSchema = ( + powerType, + generateSchemaFunc, + chassis = false +) => { const [Schema, setSchema] = useState(generateSchemaFunc({})); useEffect(() => { if (powerType) { const parametersSchema = powerType.fields.reduce((schema, field) => { - if (field.required) { - schema[field.name] = Yup.string().required(`${field.label} required`); - } else { - schema[field.name] = Yup.string(); + if (!chassis || (chassis && field.scope !== "node")) { + if (field.required) { + schema[field.name] = Yup.string().required( + `${field.label} required` + ); + } else { + schema[field.name] = Yup.string(); + } } return schema; }, {}); const newSchema = generateSchemaFunc(parametersSchema); setSchema(newSchema); } - }, [generateSchemaFunc, powerType]); + }, [chassis, generateSchemaFunc, powerType]); return Schema; }; diff --git a/ui/src/app/machines/components/PowerTypeFields/PowerTypeFields.js b/ui/src/app/machines/components/PowerTypeFields/PowerTypeFields.js index 23bb45b9c1..c7a680971f 100644 --- a/ui/src/app/machines/components/PowerTypeFields/PowerTypeFields.js +++ b/ui/src/app/machines/components/PowerTypeFields/PowerTypeFields.js @@ -3,7 +3,43 @@ import React from "react"; import FormikField from "app/base/components/FormikField"; +const generateFields = (powerTypeFields, forChassis) => { + // When adding a chassis we need only a subset of fields that aren't specific + // to individual nodes. + const fields = forChassis + ? powerTypeFields.filter((field) => field.scope !== "node") + : [...powerTypeFields]; + + return fields.map((field) => { + const { choices, field_type, label, name, required } = field; + return ( + ({ + key: `${name}-${choice[0]}`, + label: choice[1], + value: choice[0], + })) + : undefined + } + required={required} + type={ + (field_type === "string" && "text") || + (field_type === "password" && "password") || + undefined + } + /> + ); + }); +}; + export const PowerTypeFields = ({ + forChassis = false, formikProps, powerTypes, selectedPowerType, @@ -32,26 +68,7 @@ export const PowerTypeFields = ({ }} required /> - {powerType && - powerType.fields.map((field) => ( - ({ - key: `${field.name}-${choice[0]}`, - label: choice[1], - value: choice[0], - })) - : undefined - } - required={field.required} - type={field.field_type === "password" ? "password" : "text"} - /> - ))} + {powerType && generateFields(powerType.fields, forChassis)} ); }; diff --git a/ui/src/app/machines/components/PowerTypeFields/PowerTypeFields.test.js b/ui/src/app/machines/components/PowerTypeFields/PowerTypeFields.test.js index 1c0bff3357..47e2e4ca28 100644 --- a/ui/src/app/machines/components/PowerTypeFields/PowerTypeFields.test.js +++ b/ui/src/app/machines/components/PowerTypeFields/PowerTypeFields.test.js @@ -6,6 +6,67 @@ import React from "react"; import PowerTypeFields from "../PowerTypeFields"; describe("PowerTypeFields", () => { + it("gives fields the correct types", async () => { + const formikProps = { + setFieldValue: jest.fn(), + setFieldTouched: jest.fn(), + }; + const powerTypes = [ + { + name: "fake_power_type", + description: "This is not real", + fields: [ + { + name: "field1", + label: "String", + field_type: "string", + }, + { + name: "field2", + label: "Password", + field_type: "password", + }, + { + name: "field3", + label: "Choice", + field_type: "choice", + choices: [ + ["choice1", "Choice 1"], + ["choice2", "Choice 2"], + ], + }, + ], + }, + ]; + const wrapper = mount( + + + + ); + + await act(async () => { + wrapper + .find("select[name='power_type']") + .props() + .onChange({ target: { name: "power_type", value: "fake_power_type" } }); + }); + wrapper.update(); + + expect( + wrapper.find("Input[name='power_parameters.field1']").props().type + ).toBe("text"); + expect( + wrapper.find("Input[name='power_parameters.field2']").props().type + ).toBe("password"); + expect( + wrapper.find("Select[name='power_parameters.field3']").props().type + ).toBe(undefined); + }); + it("correctly generates power options from power type", async () => { const formikProps = { setFieldValue: jest.fn(), @@ -91,4 +152,55 @@ describe("PowerTypeFields", () => { .text() ).toBe("Choice 2"); }); + + it("does not show node fields if using chassis power types", async () => { + const formikProps = { + setFieldValue: jest.fn(), + setFieldTouched: jest.fn(), + }; + const powerTypes = [ + { + name: "power_type", + description: "Power", + fields: [ + { + name: "field1", + label: "Label1", + scope: "bmc", + }, + { + name: "field2", + label: "Label2", + scope: "node", + }, + ], + }, + ]; + const wrapper = mount( + + + + ); + + // Select the power type from the dropdown + await act(async () => { + wrapper + .find("select[name='power_type']") + .props() + .onChange({ target: { name: "power_type", value: "power_type" } }); + }); + wrapper.update(); + + expect(wrapper.find("Input[name='power_parameters.field1']").exists()).toBe( + true + ); + expect(wrapper.find("Input[name='power_parameters.field2']").exists()).toBe( + false + ); + }); }); diff --git a/ui/src/app/machines/views/AddChassis/AddChassisForm/AddChassisForm.js b/ui/src/app/machines/views/AddChassis/AddChassisForm/AddChassisForm.js index a2803a4629..51533a9e5a 100644 --- a/ui/src/app/machines/views/AddChassis/AddChassisForm/AddChassisForm.js +++ b/ui/src/app/machines/views/AddChassis/AddChassisForm/AddChassisForm.js @@ -19,7 +19,7 @@ import { usePowerParametersSchema, useWindowTitle, } from "app/base/hooks"; -import { trimPowerParameters } from "app/utils"; +import { formatPowerParameters } from "app/utils"; import AddChassisFormFields from "../AddChassisFormFields"; import FormCard from "app/base/components/FormCard"; import FormikForm from "app/base/components/FormikForm"; @@ -69,7 +69,8 @@ export const AddChassisForm = () => { const ChassisSchema = usePowerParametersSchema( powerType, - generateChassisSchema + generateChassisSchema, + true ); const allPowerParameters = useAllPowerParameters(chassisPowerTypes); @@ -107,7 +108,11 @@ export const AddChassisForm = () => { const params = { chassis_type: values.power_type, domain: values.domain, - ...trimPowerParameters(powerType, values.power_parameters), + ...formatPowerParameters( + powerType, + values.power_parameters, + true + ), }; dispatch(machineActions.addChassis(params)); setSavingChassis(params.hostname || "chassis"); diff --git a/ui/src/app/machines/views/AddChassis/AddChassisForm/AddChassisForm.test.js b/ui/src/app/machines/views/AddChassis/AddChassisForm/AddChassisForm.test.js index efa3b11451..1a52ad5695 100644 --- a/ui/src/app/machines/views/AddChassis/AddChassisForm/AddChassisForm.test.js +++ b/ui/src/app/machines/views/AddChassis/AddChassisForm/AddChassisForm.test.js @@ -1,3 +1,4 @@ +import { act } from "react-dom/test-utils"; import { MemoryRouter } from "react-router-dom"; import { mount } from "enzyme"; import { Provider } from "react-redux"; @@ -49,6 +50,79 @@ describe("AddChassisForm", () => { ], chassis: true, }, + { + driver_type: "power", + name: "vmware", + description: "VMware", + fields: [ + { + name: "power_vm_name", + label: "VM Name (if UUID unknown)", + required: false, + field_type: "string", + choices: [], + default: "", + scope: "node", + }, + { + name: "power_uuid", + label: "VM UUID (if known)", + required: false, + field_type: "string", + choices: [], + default: "", + scope: "node", + }, + { + name: "power_address", + label: "VMware IP", + required: true, + field_type: "string", + choices: [], + default: "", + scope: "bmc", + }, + { + name: "power_user", + label: "VMware username", + required: true, + field_type: "string", + choices: [], + default: "", + scope: "bmc", + }, + { + name: "power_pass", + label: "VMware password", + required: true, + field_type: "password", + choices: [], + default: "", + scope: "bmc", + }, + { + name: "power_port", + label: "VMware API port (optional)", + required: false, + field_type: "string", + choices: [], + default: "", + scope: "bmc", + }, + { + name: "power_protocol", + label: "VMware API protocol (optional)", + required: false, + field_type: "string", + choices: [], + default: "", + scope: "bmc", + }, + ], + missing_packages: [], + chassis: true, + queryable: true, + }, ], loaded: true, }, @@ -101,4 +175,63 @@ describe("AddChassisForm", () => { ); expect(wrapper.find("Spinner").length).toBe(1); }); + + it("correctly dispatches action to add chassis", async () => { + const state = { ...initialState }; + const store = mockStore(state); + const wrapper = mount( + + + + + + ); + + // Select vmware from power types dropdown + await act(async () => { + wrapper + .find("select[name='power_type']") + .props() + .onChange({ target: { name: "power_type", value: "vmware" } }); + }); + wrapper.update(); + + // Submit the form with unformatted power parameters + await act(async () => + wrapper + .find("Formik") + .props() + .onSubmit({ + domain: "maas", + power_parameters: { + power_address: "192.168.1.1", + power_pass: "secret", + power_port: "8000", + power_protocol: "abc123", + power_user: "user1", + }, + power_type: "vmware", + }) + ); + + // Expect the power_id param to be removed when action is dispatched. + expect( + store.getActions().find((action) => action.type === "ADD_MACHINE_CHASSIS") + ).toStrictEqual({ + type: "ADD_MACHINE_CHASSIS", + payload: { + params: { + chassis_type: "vmware", + domain: "maas", + hostname: "192.168.1.1", + password: "secret", + port: "8000", + protocol: "abc123", + username: "user1", + }, + }, + }); + }); }); diff --git a/ui/src/app/machines/views/AddChassis/AddChassisFormFields/AddChassisFormFields.js b/ui/src/app/machines/views/AddChassis/AddChassisFormFields/AddChassisFormFields.js index 553f993475..6d232fbd75 100644 --- a/ui/src/app/machines/views/AddChassis/AddChassisFormFields/AddChassisFormFields.js +++ b/ui/src/app/machines/views/AddChassis/AddChassisFormFields/AddChassisFormFields.js @@ -35,6 +35,7 @@ export const AddChassisFormFields = ({ chassisPowerTypes }) => { { ); expect(wrapper.find("AddChassisFormFields").exists()).toBe(true); }); + + it("does not show power type fields that are scoped to nodes", async () => { + const state = { ...initialState }; + state.general.powerTypes.data.push({ + name: "virsh", + description: "Virsh (virtual systems)", + fields: [ + { + name: "power_address", + label: "Address", + required: true, + field_type: "string", + choices: [], + default: "", + scope: "bmc", + }, + { + name: "power_pass", + label: "Password (optional)", + required: false, + field_type: "password", + choices: [], + default: "", + scope: "bmc", + }, + { + name: "power_id", + label: "Virsh VM ID", + required: true, + field_type: "string", + choices: [], + default: "", + scope: "node", // Should not show + }, + ], + chassis: true, + }); + const store = mockStore(state); + const wrapper = mount( + + + + + + ); + + await act(async () => { + wrapper + .find("select[name='power_type']") + .props() + .onChange({ target: { name: "power_type", value: "virsh" } }); + }); + wrapper.update(); + expect( + wrapper.find("Input[name='power_parameters.power_address']").exists() + ).toBe(true); + expect( + wrapper.find("Input[name='power_parameters.power_id']").exists() + ).toBe(false); + }); }); diff --git a/ui/src/app/machines/views/AddMachine/AddMachineForm/AddMachineForm.js b/ui/src/app/machines/views/AddMachine/AddMachineForm/AddMachineForm.js index dd843ad1a7..15f2cd7934 100644 --- a/ui/src/app/machines/views/AddMachine/AddMachineForm/AddMachineForm.js +++ b/ui/src/app/machines/views/AddMachine/AddMachineForm/AddMachineForm.js @@ -17,7 +17,7 @@ import { resourcepool as resourcePoolSelectors, zone as zoneSelectors, } from "app/base/selectors"; -import { trimPowerParameters } from "app/utils"; +import { formatPowerParameters } from "app/utils"; import { useAddMessage, useAllPowerParameters, @@ -166,7 +166,7 @@ export const AddMachineForm = () => { hostname: values.hostname, min_hwe_kernel: values.min_hwe_kernel, pool: resourcePools.find((pool) => pool.name === values.pool), - power_parameters: trimPowerParameters( + power_parameters: formatPowerParameters( powerType, values.power_parameters ), diff --git a/ui/src/app/utils/formatPowerParameters.js b/ui/src/app/utils/formatPowerParameters.js new file mode 100644 index 0000000000..c565bed939 --- /dev/null +++ b/ui/src/app/utils/formatPowerParameters.js @@ -0,0 +1,42 @@ +/** + * Formats power parameters by what is expected by the api. Also, React expects + * controlled inputs to have some associated state. Because the power parameters + * are dynamic and dependent on what power type is selected, the form is + * initialised with all possible power parameters from all power types. Before + * the action is dispatched, the power parameters are trimmed to only those + * relevant to the selected power type. + * + * @param {Object} powerType - selected power type + * @param {Object} powerParameters - all power parameters entered in Formik form + * @returns {Object} power parameters relevant to selected power type + */ + +const chassisParameterMap = new Map([ + ["power_address", "hostname"], + ["power_pass", "password"], + ["power_port", "port"], + ["power_protocol", "protocol"], + ["power_user", "username"], +]); + +export const formatPowerParameters = ( + powerType, + powerParameters, + chassis = false +) => { + const formattedParameters = {}; + if (powerType && powerType.fields) { + powerType.fields.forEach((field) => { + if (chassis) { + if (field.scope !== "node") { + const fieldName = + (chassis && chassisParameterMap.get(field.name)) || field.name; + formattedParameters[fieldName] = powerParameters[field.name]; + } + } else { + formattedParameters[field.name] = powerParameters[field.name]; + } + }); + } + return formattedParameters; +}; diff --git a/ui/src/app/utils/formatPowerParameters.test.js b/ui/src/app/utils/formatPowerParameters.test.js new file mode 100644 index 0000000000..4793b98df4 --- /dev/null +++ b/ui/src/app/utils/formatPowerParameters.test.js @@ -0,0 +1,47 @@ +import { formatPowerParameters } from "./formatPowerParameters"; + +describe("formatPowerParameters", () => { + it("trims power parameters object to those relevant to power type", () => { + const powerType = { + fields: [{ name: "field1" }, { name: "field2" }], + }; + const powerParameters = { + field1: "value1", + field2: "value2", + field3: "value3", + }; + expect(formatPowerParameters(powerType, powerParameters)).toStrictEqual({ + field1: "value1", + field2: "value2", + }); + }); + + it("can rename parameters for when adding a chassis", () => { + const powerType = { + fields: [ + { name: "power_address" }, + { name: "power_pass" }, + { name: "power_port" }, + { name: "power_protocol" }, + { name: "power_user" }, + ], + }; + + const powerParameters = { + power_address: "value1", + power_pass: "value2", + power_port: "value3", + power_protocol: "value4", + power_user: "value5", + }; + expect( + formatPowerParameters(powerType, powerParameters, true) + ).toStrictEqual({ + hostname: "value1", + password: "value2", + port: "value3", + protocol: "value4", + username: "value5", + }); + }); +}); diff --git a/ui/src/app/utils/index.js b/ui/src/app/utils/index.js index 37fe927fb8..a8f2983cd4 100644 --- a/ui/src/app/utils/index.js +++ b/ui/src/app/utils/index.js @@ -8,4 +8,4 @@ export { groupAsMap } from "./groupAsMap"; export { isVersionNewer } from "./isVersionNewer"; export { kebabToCamelCase } from "./kebabToCamelCase"; export { simpleSortByKey } from "./simpleSortByKey"; -export { trimPowerParameters } from "./trimPowerParameters"; +export { formatPowerParameters } from "./formatPowerParameters"; diff --git a/ui/src/app/utils/trimPowerParameters.js b/ui/src/app/utils/trimPowerParameters.js deleted file mode 100644 index 8283f8ed86..0000000000 --- a/ui/src/app/utils/trimPowerParameters.js +++ /dev/null @@ -1,20 +0,0 @@ -/** - * React expects controlled inputs to have some associated state. Because the - * power parameters are dynamic and dependent on what power type is selected, - * the form is initialised with all possible power parameters from all power - * types. Before the create machine action is dispatched, the power parameters - * are trimmed to only those relevant to the selected power type. - * - * @param {Object} powerType - selected power type - * @param {Object} powerParameters - all power parameters entered in Formik form - * @returns {Object} power parameters relevant to selected power type - */ -export const trimPowerParameters = (powerType, powerParameters) => { - const trimmedParameters = {}; - if (powerType && powerType.fields) { - powerType.fields.forEach((field) => { - trimmedParameters[field.name] = powerParameters[field.name]; - }); - } - return trimmedParameters; -};