Fix required options accepting a stale configured default

Treat a `config_default` that is not among the values returned by the
provider as no value, so the required field blocks the wizard instead
of silently passing validation. Options with no list of values keep
their configured default.

Signed-off-by: Mihaela Balutoiu <mbalutoiu@cloudbasesolutions.com>

src/@types/Field.ts

@@ -57,6 +57,55 @@ export type Field = {
   groupName?: string;
 };
 
+const enumItemMatchesValue = (item: EnumItem, value: any): boolean => {
+  if (isEnumSeparator(item)) {
+    return false;
+  }
+  if (typeof item !== "object") {
+    return item === value;
+  }
+  return (
+    (item.id != null && item.id === value) ||
+    (item.value !== undefined && item.value === value) ||
+    (item.name !== undefined && item.name === value) ||
+    (item.label !== undefined && item.label === value)
+  );
+};
+
+export const findEnumItem = (
+  enumItems: EnumItem[] | null | undefined,
+  value: any,
+): EnumItem | undefined => {
+  if (!enumItems?.length || value === undefined || value === null) {
+    return undefined;
+  }
+  const searchedValue =
+    typeof value === "object" && value.id != null ? value.id : value;
+  return enumItems.find(item => enumItemMatchesValue(item, searchedValue));
+};
+
+export type ResolvedFieldDefault =
+  | { hasValue: false }
+  | { hasValue: true; value: any };
+
+export const resolveFieldDefault = (field: Field): ResolvedFieldDefault => {
+  const fieldDefault = field.default;
+  if (fieldDefault === undefined || fieldDefault === null) {
+    return { hasValue: false };
+  }
+  if (!field.enum?.length) {
+    return { hasValue: true, value: fieldDefault };
+  }
+  const matchedItem: any = findEnumItem(field.enum, fieldDefault);
+  if (!matchedItem) {
+    return { hasValue: false };
+  }
+  return {
+    hasValue: true,
+    value: matchedItem.id != null ? matchedItem.id : fieldDefault,
+  };
+};
+
 const migrationImageOsTypes = ["windows", "linux"];
 
 class FieldHelper {

src/components/modules/WizardModule/WizardOptions/WizardOptions.spec.tsx

@@ -17,7 +17,9 @@ import React from "react";
 import { render } from "@testing-library/react";
 import { MINION_POOL_MOCK } from "@tests/mocks/MinionPoolMock";
 
-import WizardOptions from "./";
+import WizardOptions, { findInvalidFields } from "./";
+
+import type { Field } from "@src/@types/Field";
 
 jest.mock("@src/plugins/default/ContentPlugin", () => jest.fn(() => null));
 jest.mock("@src/utils/Config", () => ({
@@ -48,3 +50,208 @@ describe("WizardOptions", () => {
     expect(getByText("Target Minion Pool")).toBeTruthy();
   });
 });
+
+describe("WizardOptions.findInvalidFields", () => {
+  const names = (data: any, schema: Field[]) =>
+    findInvalidFields(data, schema).map(f => f.name);
+
+  it("returns nothing for an empty schema", () => {
+    expect(findInvalidFields({}, [])).toEqual([]);
+  });
+
+  it("ignores optional fields", () => {
+    const schema: Field[] = [{ name: "description", type: "string" }];
+    expect(names({}, schema)).toEqual([]);
+  });
+
+  it("reports a required field with neither a value nor a default", () => {
+    const schema: Field[] = [
+      { name: "migration_image", type: "string", required: true },
+    ];
+    expect(names({}, schema)).toEqual(["migration_image"]);
+  });
+
+  it("accepts a required field whose default is one of the available values", () => {
+    const schema: Field[] = [
+      {
+        name: "migration_image",
+        type: "string",
+        required: true,
+        enum: [{ id: "windows-image-id", name: "Windows" }],
+        default: "windows-image-id",
+      },
+    ];
+    expect(names({}, schema)).toEqual([]);
+  });
+
+  it("reports a required field whose default is no longer an available value", () => {
+    const schema: Field[] = [
+      {
+        name: "migration_image",
+        type: "string",
+        required: true,
+        enum: [{ id: "valid-image-id", name: "Windows Server" }],
+        default: "deleted-image-id",
+      },
+    ];
+    expect(names({}, schema)).toEqual(["migration_image"]);
+  });
+
+  it("reports a required field whose stale default was already discarded", () => {
+    const schema: Field[] = [
+      {
+        name: "migration_image",
+        type: "string",
+        required: true,
+        enum: [{ id: "valid-image-id", name: "Windows Server" }],
+        default: null,
+      },
+    ];
+    expect(names({}, schema)).toEqual(["migration_image"]);
+  });
+
+  it("ignores an optional field with a stale default", () => {
+    const schema: Field[] = [
+      {
+        name: "migration_image",
+        type: "string",
+        enum: [{ id: "valid-image-id", name: "Windows Server" }],
+        default: "deleted-image-id",
+      },
+    ];
+    expect(names({}, schema)).toEqual([]);
+  });
+
+  it("uses the value from the data over the field default", () => {
+    const schema: Field[] = [
+      {
+        name: "migration_image",
+        type: "string",
+        required: true,
+        enum: [{ id: "valid-image-id", name: "Windows Server" }],
+        default: "deleted-image-id",
+      },
+    ];
+    expect(names({ migration_image: "valid-image-id" }, schema)).toEqual([]);
+  });
+
+  it("accepts a required boolean set to false", () => {
+    const schema: Field[] = [
+      { name: "list_all_networks", type: "boolean", required: true },
+    ];
+    expect(names({ list_all_networks: false }, schema)).toEqual([]);
+  });
+
+  it("accepts a required boolean whose default is false", () => {
+    const schema: Field[] = [
+      {
+        name: "list_all_networks",
+        type: "boolean",
+        required: true,
+        default: false,
+      },
+    ];
+    expect(names({}, schema)).toEqual([]);
+  });
+
+  it("accepts a required integer set to 0", () => {
+    const schema: Field[] = [
+      { name: "disk_size", type: "integer", required: true },
+    ];
+    expect(names({ disk_size: 0 }, schema)).toEqual([]);
+  });
+
+  it("accepts a required integer whose default is 0 with an empty values list", () => {
+    const schema: Field[] = [
+      {
+        name: "disk_size",
+        type: "integer",
+        required: true,
+        enum: [],
+        default: 0,
+      },
+    ];
+    expect(names({}, schema)).toEqual([]);
+  });
+
+  it.each([
+    ["an empty string", ""],
+    ["null", null],
+  ])("reports a required field set to %s", (_label, value) => {
+    const schema: Field[] = [
+      { name: "migration_image", type: "string", required: true },
+    ];
+    expect(names({ migration_image: value }, schema)).toEqual([
+      "migration_image",
+    ]);
+  });
+
+  it("reports a stale default on a required property of an object field", () => {
+    const schema: Field[] = [
+      {
+        name: "migr_image_map",
+        type: "object",
+        properties: [
+          {
+            name: "linux",
+            type: "string",
+            required: true,
+            enum: [{ id: "valid-image-id", name: "Ubuntu" }],
+            default: "deleted-image-id",
+          },
+          {
+            name: "windows",
+            type: "string",
+            enum: [{ id: "windows-image-id", name: "Windows" }],
+            default: "windows-image-id",
+          },
+        ],
+      },
+    ];
+    expect(names({}, schema)).toEqual(["linux"]);
+  });
+
+  it("accepts an object field property that has a value under its group", () => {
+    const schema: Field[] = [
+      {
+        name: "migr_image_map",
+        type: "object",
+        properties: [
+          {
+            name: "linux",
+            type: "string",
+            required: true,
+            enum: [{ id: "valid-image-id", name: "Ubuntu" }],
+            default: "deleted-image-id",
+          },
+        ],
+      },
+    ];
+    expect(
+      names({ migr_image_map: { linux: "valid-image-id" } }, schema),
+    ).toEqual([]);
+  });
+
+  it("validates the required properties of the selected sub field", () => {
+    const schema: Field[] = [
+      {
+        name: "replica_export_mechanism",
+        type: "string",
+        enum: ["direct", "image"],
+        subFields: [
+          {
+            name: "direct_options",
+            properties: [{ name: "direct_volume", required: true }],
+          },
+          {
+            name: "image_options",
+            properties: [{ name: "export_image", required: true }],
+          },
+        ],
+      },
+    ];
+    expect(names({ replica_export_mechanism: "image" }, schema)).toEqual([
+      "export_image",
+    ]);
+  });
+});

src/components/modules/WizardModule/WizardOptions/WizardOptions.tsx

@@ -20,6 +20,7 @@ import { CSSTransition } from "react-transition-group";
 import styled from "styled-components";
 
 import { MinionPool } from "@src/@types/MinionPool";
+import { resolveFieldDefault } from "@src/@types/Field";
 import { ThemePalette, ThemeProps } from "@src/components/Theme";
 import FieldInput from "@src/components/ui/FieldInput";
 import StatusImage from "@src/components/ui/StatusComponents/StatusImage";
@@ -130,14 +131,21 @@ export const shouldRenderField = (field: Field) =>
     (field.enum && field.enum.length && field.enum.length > 0)) &&
   (field.type !== "object" || field.properties);
 
+const hasValue = (value: any): boolean =>
+  value !== undefined && value !== null && value !== "";
+
 export const findInvalidFields = (data: any, schema: Field[]): Field[] => {
   const isInvalid = (field: Field): boolean => {
-    if (field.groupName && data[field.groupName]?.[field.name] !== undefined) {
-      return !data[field.groupName][field.name];
-    } else if (data[field.name] !== undefined) {
-      return !data[field.name];
+    if (
+      field.groupName &&
+      data?.[field.groupName]?.[field.name] !== undefined
+    ) {
+      return !hasValue(data[field.groupName][field.name]);
+    } else if (data?.[field.name] !== undefined) {
+      return !hasValue(data[field.name]);
     } else {
-      return !field.default;
+      const resolvedDefault = resolveFieldDefault(field);
+      return !resolvedDefault.hasValue || !hasValue(resolvedDefault.value);
     }
   };
 

src/plugins/default/OptionsSchemaPlugin.ts

@@ -14,7 +14,12 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import Utils from "@src/utils/ObjectUtils";
 
-import { Field, EnumItem, isEnumSeparator } from "@src/@types/Field";
+import {
+  Field,
+  EnumItem,
+  isEnumSeparator,
+  resolveFieldDefault,
+} from "@src/@types/Field";
 import type { OptionValues, StorageMap } from "@src/@types/Endpoint";
 import type { SchemaProperties, SchemaDefinitions } from "@src/@types/Schema";
 import type { NetworkMap } from "@src/@types/Network";
@@ -26,10 +31,22 @@ import { defaultSchemaToFields } from "./ConnectionSchemaPlugin";
 
 const migrationImageOsTypes = ["windows", "linux"];
 
+export const discardStaleEnumDefault = (field: Field) => {
+  if (field.default === undefined || field.default === null) {
+    return;
+  }
+  if (!field.enum?.length) {
+    return;
+  }
+  if (!resolveFieldDefault(field).hasValue) {
+    field.default = null;
+  }
+};
+
 export const defaultFillFieldValues = (field: Field, option: OptionValues) => {
   if (field.type === "string") {
     field.enum = [...option.values] as EnumItem[];
-    if (option.config_default) {
+    if (option.config_default !== undefined && option.config_default !== null) {
       field.default =
         typeof option.config_default === "string"
           ? option.config_default
@@ -37,6 +54,7 @@ export const defaultFillFieldValues = (field: Field, option: OptionValues) => {
             ? option.config_default.id
             : String(option.config_default);
     }
+    discardStaleEnumDefault(field);
   }
   if (field.type === "array") {
     field.enum = [...option.values] as EnumItem[];
@@ -56,6 +74,7 @@ export const defaultFillFieldValues = (field: Field, option: OptionValues) => {
     if (option.config_default != null) {
       field.default = Number(option.config_default);
     }
+    discardStaleEnumDefault(field);
   }
 };
 
@@ -76,6 +95,7 @@ export const removeExportImageFieldValues = (field: Field) => {
       }
       return isLinux;
     });
+    discardStaleEnumDefault(field);
   }
 };
 
@@ -124,13 +144,15 @@ export const defaultFillMigrationImageMapValues = (opts: {
       defaultValue = option.config_default[os];
     }
 
-    return {
+    const property: Field = {
       name: os,
       type: "string",
       enum: values,
       default: defaultValue,
       required: os === "linux" || (requiresWindowsImage && os === "windows"),
     };
+    discardStaleEnumDefault(property);
+    return property;
   });
   return true;
 };

src/stores/WizardStore.ts

@@ -20,6 +20,8 @@ import type {
   InstanceScript,
   UserScriptTarget,
 } from "@src/@types/Instance";
+import { resolveFieldDefault } from "@src/@types/Field";
+
 import type { Field } from "@src/@types/Field";
 import type { NetworkMap } from "@src/@types/Network";
 import type { StorageMap } from "@src/@types/Endpoint";
@@ -112,33 +114,11 @@ class WizardStore {
       if (parentData[field.name] !== undefined) {
         return { should: false };
       }
-      const fieldDefault = field.default;
-      if (fieldDefault == null) {
-        return { should: false };
-      }
-      if (field.enum) {
-        const isDefaultInEnum = field.enum.find(item => {
-          const enumItem: any = item;
-          if (fieldDefault.id != null) {
-            return enumItem.id != null
-              ? enumItem.id === fieldDefault.id
-              : enumItem === fieldDefault.id;
-          }
-          return enumItem.id != null
-            ? enumItem.id === fieldDefault || enumItem.name === fieldDefault
-            : enumItem === fieldDefault || enumItem.value === fieldDefault;
-        });
-
-        // Don't use the default if it can't be found in the enum list.
-        if (isDefaultInEnum) {
-          const matchedItem: any = isDefaultInEnum;
-          const value = matchedItem.id != null ? matchedItem.id : field.default;
-          return { should: true, value };
-        }
-      } else {
-        return { should: true, value: field.default };
-      }
-      return { should: false };
+      // Don't use the default if it can't be found in the list of values.
+      const resolvedDefault = resolveFieldDefault(field);
+      return resolvedDefault.hasValue
+        ? { should: true, value: resolvedDefault.value }
+        : { should: false };
     };
 
     const setObjectDefault = (