Add validation on instance create

.github/workflows/commits.yaml

@@ -10,7 +10,7 @@ jobs:
     permissions:
       pull-requests: read   # to get list of commits from the PR
     name: Canonical CLA signed and Signed-off-by (DCO)
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-latest
     steps:
     - name: Check if CLA signed
       uses: canonical/has-signed-canonical-cla@v1

src/components/forms/DiskDeviceFormCustom.tsx

@@ -12,6 +12,7 @@ import { getConfigurationRowBase } from "components/ConfigurationRow";
 import DetachDiskDeviceBtn from "pages/instances/actions/DetachDiskDeviceBtn";
 import classnames from "classnames";
 import { LxdStorageVolume } from "types/storage";
+import { isDiskDeviceMountPointMissing } from "util/instanceValidation";
 
 interface Props {
   formik: SharedFormikTypes;
@@ -39,6 +40,9 @@ const DiskDeviceFormCustom: FC<Props> = ({
       source: volume.name,
     });
     void formik.setFieldValue("devices", copy);
+
+    const name = `devices.${copy.length - 1}.path`;
+    setTimeout(() => document.getElementById(name)?.focus(), 100);
   };
 
   const changeVolume = (
@@ -96,7 +100,7 @@ const DiskDeviceFormCustom: FC<Props> = ({
       getConfigurationRowBase({
         className: "no-border-top inherited-with-form",
         configuration: (
-          <Label forId={`volume-${index}-pool`}>Pool / volume</Label>
+          <Label forId={`devices.${index}.pool`}>Pool / volume</Label>
         ),
         inherited: (
           <div className="custom-disk-volume-source">
@@ -119,7 +123,7 @@ const DiskDeviceFormCustom: FC<Props> = ({
                 project={project}
                 setValue={(volume) => changeVolume(volume, formVolume, index)}
                 buttonProps={{
-                  id: `volume-${index}-pool`,
+                  id: `devices.${index}.pool`,
                   appearance: "base",
                   className: "u-no-margin--bottom",
                 }}
@@ -134,11 +138,12 @@ const DiskDeviceFormCustom: FC<Props> = ({
     );
 
     if (formVolume.path !== undefined) {
+      const hasError = isDiskDeviceMountPointMissing(formik, index);
       rows.push(
         getConfigurationRowBase({
           className: "no-border-top inherited-with-form",
           configuration: (
-            <Label forId={`volume-${index}-path`} required>
+            <Label forId={`devices.${index}.path`} required>
               Mount point
             </Label>
           ),
@@ -148,7 +153,8 @@ const DiskDeviceFormCustom: FC<Props> = ({
             </div>
           ) : (
             <Input
-              id={`volume-${index}-path`}
+              id={`devices.${index}.path`}
+              name={`devices.${index}.path`}
               onBlur={formik.handleBlur}
               onChange={(e) => {
                 void formik.setFieldValue(
@@ -159,7 +165,8 @@ const DiskDeviceFormCustom: FC<Props> = ({
               value={formVolume.path}
               type="text"
               placeholder="Enter full path (e.g. /data)"
-              className="u-no-margin--bottom"
+              className={hasError ? undefined : "u-no-margin--bottom"}
+              error={hasError ? "Path is required" : undefined}
             />
           ),
           override: "",
@@ -171,7 +178,7 @@ const DiskDeviceFormCustom: FC<Props> = ({
       getConfigurationRowBase({
         className: "no-border-top inherited-with-form",
         configuration: (
-          <Label forId={`volume-${index}-read-limit`}>Read limit</Label>
+          <Label forId={`devices.${index}.limits.read`}>Read limit</Label>
         ),
         inherited: isReadOnly ? (
           <div className="mono-font">
@@ -184,7 +191,8 @@ const DiskDeviceFormCustom: FC<Props> = ({
         ) : (
           <div className="custom-disk-device-limits">
             <Input
-              id={`volume-${index}-read-limit`}
+              id={`devices.${index}.limits.read`}
+              name={`devices.${index}.limits.read`}
               onBlur={formik.handleBlur}
               onChange={(e) => {
                 void formik.setFieldValue(
@@ -208,7 +216,7 @@ const DiskDeviceFormCustom: FC<Props> = ({
       getConfigurationRowBase({
         className: "no-border-top inherited-with-form",
         configuration: (
-          <Label forId={`volume-${index}-write-limit`}>Write limit</Label>
+          <Label forId={`devices.${index}.limits.write`}>Write limit</Label>
         ),
         inherited: isReadOnly ? (
           <div className="mono-font">
@@ -221,7 +229,8 @@ const DiskDeviceFormCustom: FC<Props> = ({
         ) : (
           <div className="custom-disk-device-limits">
             <Input
-              id={`volume-${index}-write-limit`}
+              id={`devices.${index}.limits.write`}
+              name={`devices.${index}.limits.write`}
               onBlur={formik.handleBlur}
               onChange={(e) => {
                 void formik.setFieldValue(

src/components/forms/DiskDeviceFormRoot.tsx

@@ -12,6 +12,7 @@ import DiskSizeSelector from "components/forms/DiskSizeSelector";
 import { LxdStoragePool } from "types/storage";
 import { LxdProfile } from "types/profile";
 import { removeDevice } from "util/formDevices";
+import { hasNoRootDisk } from "util/instanceValidation";
 
 interface Props {
   formik: SharedFormikTypes;
@@ -141,6 +142,14 @@ const DiskDeviceFormRoot: FC<Props> = ({
           }),
         ]}
       />
+      {hasNoRootDisk(formik.values, profiles) && (
+        <div className="is-error ">
+          <p className="p-form-validation__message">
+            <strong>Error:</strong> Missing root storage. Create an override, or
+            add a profile with root storage.
+          </p>
+        </div>
+      )}
     </>
   );
 };

src/components/forms/FormMenuItem.tsx

@@ -1,19 +1,22 @@
 import React, { FC } from "react";
 import { slugify } from "util/slugify";
 import { Button } from "@canonical/react-components";
+import classnames from "classnames";
 
 interface Props {
   active: string;
   setActive: (item: string) => void;
   label: string;
   disableReason?: string;
+  hasError?: boolean;
 }
 
 const FormMenuItem: FC<Props> = ({
   active,
   setActive,
   label,
   disableReason,
+  hasError,
 }) => {
   if (disableReason) {
     return (
@@ -29,7 +32,11 @@ const FormMenuItem: FC<Props> = ({
     );
   }
   return (
-    <li className="p-side-navigation__item">
+    <li
+      className={classnames("p-side-navigation__item", {
+        "has-error": hasError,
+      })}
+    >
       <a
         className="p-side-navigation__link"
         onClick={() => setActive(label)}

src/components/forms/NetworkDevicesForm.tsx

@@ -3,7 +3,6 @@ import {
   Button,
   Icon,
   Input,
-  Label,
   Select,
   Tooltip,
   useNotify,
@@ -20,6 +19,7 @@ import { getConfigurationRowBase } from "components/ConfigurationRow";
 import Loader from "components/Loader";
 import { figureInheritedNetworks } from "util/instanceConfigInheritance";
 import { CustomNetworkDevice } from "util/formDevices";
+import { isNicDeviceNameMissing } from "util/instanceValidation";
 
 interface Props {
   formik: SharedFormikTypes;
@@ -67,8 +67,11 @@ const NetworkDevicesForm: FC<Props> = ({ formik, project }) => {
 
   const addNetwork = () => {
     const copy = [...formik.values.devices];
-    copy.push({ type: "nic", name: "" });
+    copy.push({ type: "nic", name: "", network: networks[0]?.name ?? "" });
     void formik.setFieldValue("devices", copy);
+
+    const name = `devices.${copy.length - 1}.name`;
+    setTimeout(() => document.getElementById(name)?.focus(), 100);
   };
 
   const getNetworkOptions = () => {
@@ -83,6 +86,7 @@ const NetworkDevicesForm: FC<Props> = ({ formik, project }) => {
     options.unshift({
       label: networks.length === 0 ? "No networks available" : "Select option",
       value: "",
+      disabled: true,
     });
     return options;
   };
@@ -134,27 +138,28 @@ or remove the originating item"
 
           return getConfigurationRowBase({
             configuration: (
-              <Label forId={`networkDevice${index}`}>
-                <b>
-                  {isReadOnly || device.type === "custom-nic" ? (
-                    device.name
-                  ) : (
-                    <Input
-                      label="Device name"
-                      required
-                      name={`devices.${index}.name`}
-                      id={`networkName${index}`}
-                      onBlur={formik.handleBlur}
-                      onChange={formik.handleChange}
-                      value={
-                        (formik.values.devices[index] as LxdNicDevice).name
-                      }
-                      type="text"
-                      placeholder="Enter name"
-                    />
-                  )}
-                </b>
-              </Label>
+              <>
+                {isReadOnly || device.type === "custom-nic" ? (
+                  device.name
+                ) : (
+                  <Input
+                    label="Device name"
+                    required
+                    name={`devices.${index}.name`}
+                    id={`devices.${index}.name`}
+                    onBlur={formik.handleBlur}
+                    onChange={formik.handleChange}
+                    value={(formik.values.devices[index] as LxdNicDevice).name}
+                    type="text"
+                    placeholder="Enter name"
+                    error={
+                      isNicDeviceNameMissing(formik, index)
+                        ? "Device name is required"
+                        : undefined
+                    }
+                  />
+                )}
+              </>
             ),
             inherited: "",
             override:
@@ -173,7 +178,7 @@ or remove the originating item"
                     <Select
                       label="Network"
                       name={`devices.${index}.network`}
-                      id={`networkDevice${index}`}
+                      id={`devices.${index}.network`}
                       onBlur={formik.handleBlur}
                       onChange={formik.handleChange}
                       value={

src/pages/instances/CreateInstance.tsx

@@ -11,7 +11,7 @@ import {
 import { useFormik } from "formik";
 import * as Yup from "yup";
 import { createInstance, startInstance } from "api/instances";
-import { useQueryClient } from "@tanstack/react-query";
+import { useQuery, useQueryClient } from "@tanstack/react-query";
 import { queryKeys } from "util/queryKeys";
 import SubmitButton from "components/SubmitButton";
 import { LxdImageType, RemoteImage } from "types/image";
@@ -66,6 +66,12 @@ import { useEventQueue } from "context/eventQueue";
 import { getInstanceName } from "util/operations";
 import NotificationRow from "components/NotificationRow";
 import BaseLayout from "components/BaseLayout";
+import { fetchProfiles } from "api/profiles";
+import {
+  hasDiskError,
+  hasNetworkError,
+  hasNoRootDisk,
+} from "util/instanceValidation";
 
 export type CreateInstanceFormValues = InstanceDetailsFormValues &
   FormDeviceValues &
@@ -213,14 +219,25 @@ const CreateInstance: FC = () => {
     }
   };
 
-  const submit = (values: CreateInstanceFormValues, shouldStart = true) => {
-    const formUrl = location.pathname + location.search;
-    navigate(`/ui/project/${project}/instances`);
+  const { data: profiles = [] } = useQuery({
+    queryKey: [queryKeys.profiles],
+    queryFn: () => fetchProfiles(project),
+  });
 
-    const instancePayload = values.yaml
+  const submit = (values: CreateInstanceFormValues, shouldStart = true) => {
+    const instancePayload: Partial<LxdInstance> = values.yaml
       ? yamlToObject(values.yaml)
       : getPayload(values);
 
+    if (hasNoRootDisk(values, profiles)) {
+      setConfigOpen(true);
+      setSection(DISK_DEVICES);
+      return;
+    }
+
+    const formUrl = location.pathname + location.search;
+    navigate(`/ui/project/${project}/instances`);
+
     createInstance(JSON.stringify(instancePayload), project, values.target)
       .then((operation) => {
         const instanceName = getInstanceName(operation.metadata);
@@ -246,7 +263,7 @@ const CreateInstance: FC = () => {
     initialValues: location.state?.retryFormValues ?? {
       instanceType: "container",
       profiles: ["default"],
-      devices: [{ type: "nic", name: "" }],
+      devices: [],
       readOnly: false,
       type: "instance",
     },
@@ -318,6 +335,11 @@ const CreateInstance: FC = () => {
     return dumpYaml(payload);
   }
 
+  const diskError = hasDiskError(formik);
+  const networkError = hasNetworkError(formik);
+  const hasErrors =
+    !formik.isValid || !formik.values.image || diskError || networkError;
+
   return (
     <BaseLayout title="Create an instance" contentClassName="create-instance">
       <Form onSubmit={formik.handleSubmit} stacked className="form">
@@ -327,6 +349,8 @@ const CreateInstance: FC = () => {
           isConfigDisabled={!formik.values.image}
           isConfigOpen={isConfigOpen}
           toggleConfigOpen={toggleMenu}
+          hasDiskError={diskError || hasNoRootDisk(formik.values, profiles)}
+          hasNetworkError={networkError}
         />
         <Row className="form-contents" key={section}>
           <Col size={12}>
@@ -393,15 +417,15 @@ const CreateInstance: FC = () => {
             </Button>
             <SubmitButton
               isSubmitting={formik.isSubmitting}
-              isDisabled={!formik.isValid || !formik.values.image}
+              isDisabled={hasErrors}
               buttonLabel="Create"
               appearance={isLocalIsoImage ? "positive" : "default"}
               onClick={() => submit(formik.values, false)}
             />
             {!isLocalIsoImage && (
               <SubmitButton
                 isSubmitting={formik.isSubmitting}
-                isDisabled={!formik.isValid || !formik.values.image}
+                isDisabled={hasErrors}
                 buttonLabel="Create and start"
                 onClick={() => submit(formik.values)}
               />