Merge user values with defaults before validation. (#5623)

Signed-off-by: Michael Nelson <[email protected]>

<!--
Before you open the request please review the following guidelines and
tips to help it be more easily integrated:

 - Describe the scope of your change - i.e. what the change does.
 - Describe any known limitations with your change.
- Please run any tests or examples that can exercise your modified code.

 Thank you for contributing!
 -->

### Description of the change

<!-- Describe the scope of your change - i.e. what the change does. -->

Updates the `validateValuesSchema` function to additionally use the
package default values, when specified, so that user values will be
confirmed as valid even if they don't set certain values that are
required by the schema but have default values specified by the package.

### Benefits

<!-- What benefits will be realized by the code change? -->

Enables the scenario outlined by @dud225 in #4803, where a helm chart is
deployed via the Helm CLI, rather than Kubeapps, and so the values set
by the user are only those actually set by the user (as opposed to when
deployed with Kubeapps where we currently incorrectly send all the
default values as user-set values - I may open a separate issue for
that, depending on a few things).

### Possible drawbacks

<!-- Describe any known limitations with your change -->

### Applicable issues

<!-- Enter any applicable Issues here (You can reference an issue using
#) -->

- fixes #4803

### Additional information

<!-- If there's anything else that's important and relevant to your pull
request, mention that information here.-->

I've not yet tested this IRL to ensure it fixes the scenario in the
issue, but will do so tomorrow.

Signed-off-by: Michael Nelson <[email protected]>

dashboard/src/actions/installedpackages.ts

@@ -243,7 +243,11 @@ export function installPackage(
             `The schema for this package is not valid. Please contact the package author. The following errors were found:\n${errorText}`,
           );
         }
-        const validation = validateValuesSchema(values, schema);
+        const validation = validateValuesSchema(
+          values,
+          schema,
+          availablePackageDetail.defaultValues,
+        );
         if (!validation.valid) {
           const errorText = validation?.errors
             ?.map(e => `  - ${e.instancePath}: ${e.message}`)
@@ -301,7 +305,11 @@ export function updateInstalledPackage(
             `The schema for this package is not valid. Please contact the package author. The following errors were found:\n${errorText}`,
           );
         }
-        const validation = validateValuesSchema(values, schema);
+        const validation = validateValuesSchema(
+          values,
+          schema,
+          availablePackageDetail.defaultValues,
+        );
         if (!validation.valid) {
           const errorText = validation?.errors
             ?.map(e => `  - ${e.instancePath}: ${e.message}`)

dashboard/src/shared/schema.test.ts

@@ -434,15 +434,67 @@ describe("validateValuesSchema", () => {
     {
       description: "Should validate a valid object",
       values: "foo: bar\n",
+      defaultValues: "foo: default\n",
       schema: {
         properties: { foo: { type: "string" } },
       },
       valid: true,
       errors: null,
     },
+    {
+      description: "Should error if required value not provided",
+      values: "foo: bar\n",
+      defaultValues: "foo: default\n",
+      schema: {
+        required: ["bar"],
+        properties: {
+          foo: { type: "string" },
+          bar: { type: "string" },
+        },
+      },
+      valid: false,
+      errors: [
+        {
+          keyword: "required",
+          instancePath: "",
+          message: "must have required property 'bar'",
+          params: { missingProperty: "bar" },
+          schemaPath: "#/required",
+        },
+      ],
+    },
+    {
+      description: "Should not error if required value has a default",
+      values: "foo: bar\n",
+      defaultValues: "foo: default\nbar: default\n",
+      schema: {
+        required: ["bar"],
+        properties: {
+          foo: { type: "string" },
+          bar: { type: "string" },
+        },
+      },
+      valid: true,
+      errors: null,
+    },
+    {
+      description: "Should not error if extra value not present in defaults is included",
+      values: "foo: bar\nelsewhere: bang",
+      defaultValues: "foo: default\nbar: default\n",
+      schema: {
+        required: ["bar"],
+        properties: {
+          foo: { type: "string" },
+          bar: { type: "string" },
+        },
+      },
+      valid: true,
+      errors: null,
+    },
     {
       description: "Should validate an invalid object",
       values: "foo: bar\n",
+      defaultValues: "foo: 1\n",
       schema: { properties: { foo: { type: "integer" } } },
       valid: false,
       errors: [
@@ -459,7 +511,7 @@ describe("validateValuesSchema", () => {
     },
   ].forEach(t => {
     it(t.description, () => {
-      const res = validateValuesSchema(t.values, t.schema);
+      const res = validateValuesSchema(t.values, t.schema, t.defaultValues);
       expect(res.valid).toBe(t.valid);
       expect(res.errors).toEqual(t.errors);
     });

dashboard/src/shared/schema.ts

@@ -5,7 +5,8 @@ import Ajv, { ErrorObject, JSONSchemaType } from "ajv";
 import { findIndex, isEmpty, set } from "lodash";
 import { DeploymentEvent, IAjvValidateResult, IBasicFormParam } from "shared/types";
 import YAML from "yaml";
-import { getPathValueInYamlNode, getPathValueInYamlNodeWithDefault, parseToJS } from "./yamlUtils";
+import * as jsonpatch from "fast-json-patch";
+import { getPathValueInYamlNode, getPathValueInYamlNodeWithDefault } from "./yamlUtils";
 
 const ajv = new Ajv({ strict: false });
 
@@ -22,7 +23,6 @@ export function retrieveBasicFormParams(
   let params: IBasicFormParam[] = [];
   if (schema?.properties && !isEmpty(schema.properties)) {
     const properties = schema.properties;
-    const requiredProperties = schema.required;
     const schemaExamples = schema.examples;
     Object.keys(properties).forEach(propertyKey => {
       const schemaProperty = properties[propertyKey] as JSONSchemaType<any>;
@@ -59,8 +59,9 @@ export function retrieveBasicFormParams(
           : undefined,
         // get the string values of the enum array
         enum: schemaProperty?.enum?.map((item: { toString: () => any }) => item?.toString() ?? ""),
-        // check if the "required" array contains the current property
-        isRequired: requiredProperties?.includes(propertyKey),
+        // We leave the validation of user values to the Helm backend, since it will take
+        // into account default values (on install) and previously set values (on upgrade).
+        isRequired: false,
         examples: examples,
         // If exists, the value that is currently deployed
         deployedValue: isLeaf
@@ -155,8 +156,18 @@ export function schemaToObject(schema?: string): JSONSchemaType<any> {
 export function validateValuesSchema(
   values: string,
   schema: JSONSchemaType<any> | any,
+  defaultValues?: string,
 ): { valid: boolean; errors: ErrorObject[] | null | undefined } {
-  const valid = ajv.validate(schema, parseToJS(values));
+  let valuesToCheck = YAML.parse(values);
+  if (defaultValues) {
+    const defaultYAML = YAML.parse(defaultValues);
+    let patches = jsonpatch.compare(defaultYAML as any, valuesToCheck as any);
+    patches = patches.filter(function (d) {
+      return ["add", "replace"].includes(d.op);
+    });
+    valuesToCheck = jsonpatch.applyPatch(defaultYAML, patches).newDocument;
+  }
+  const valid = ajv.validate(schema, valuesToCheck);
   return { valid: !!valid, errors: ajv.errors } as IAjvValidateResult;
 }