From b52093d1885a24fa19f9583ea2d282cc4575f987 Mon Sep 17 00:00:00 2001
From: Simon Kobyda <skobyda@redhat.com>
Date: Thu, 13 Jul 2023 12:06:58 +0200
Subject: [PATCH] Validate Environment variables fields in Create container
 dialog

---
 src/Env.jsx            | 54 ++++++++++++++++++++++++++++++++++++++----
 src/ImageRunModal.jsx  | 20 ++++++++++++----
 test/check-application |  9 +++++++
 3 files changed, 74 insertions(+), 9 deletions(-)

diff --git a/src/Env.jsx b/src/Env.jsx
index 1944a963a..5cd903781 100644
--- a/src/Env.jsx
+++ b/src/Env.jsx
@@ -1,13 +1,35 @@
 import React from 'react';
 import { Button } from "@patternfly/react-core/dist/esm/components/Button";
 import { FormGroup } from "@patternfly/react-core/dist/esm/components/Form";
+import { FormHelper } from "cockpit-components-form-helper.jsx";
 import { Grid } from "@patternfly/react-core/dist/esm/layouts/Grid";
 import { TextInput } from "@patternfly/react-core/dist/esm/components/TextInput";
 import { TrashIcon } from '@patternfly/react-icons';
 import cockpit from 'cockpit';
 
+import * as utils from './util.js';
+
 const _ = cockpit.gettext;
 
+export function validateEnvVar(env, key, validationFailed) {
+    const delta = { ...validationFailed };
+    delete delta[key];
+
+    switch (key) {
+    case "envKey":
+        if (utils.isEmpty(env))
+            delta.envKey = _("Key must not be empty");
+        break;
+    case "envValue":
+        if (utils.isEmpty(env))
+            delta.envValue = _("Value must not be empty");
+
+        break;
+    }
+
+    return delta;
+}
+
 const handleEnvValue = (key, value, idx, onChange, additem, itemCount, companionField) => {
     // Allow the input of KEY=VALUE separated value pairs for bulk import only if the other
     // field is not empty.
@@ -32,18 +54,40 @@ const handleEnvValue = (key, value, idx, onChange, additem, itemCount, companion
     }
 };
 
-export const EnvVar = ({ id, item, onChange, idx, removeitem, additem, itemCount }) =>
+export const EnvVar = ({ id, item, onChange, idx, removeitem, additem, itemCount, validationFailed, onValidationChange }) =>
     (
         <Grid hasGutter id={id}>
-            <FormGroup className="pf-m-6-col-on-md" label={_("Key")} fieldId={id + "-key-address"}>
+            <FormGroup className="pf-m-6-col-on-md"
+                id={id + "-key-group"}
+                label={_("Key")}
+                fieldId={id + "-key-address"}
+                isRequired
+            >
                 <TextInput id={id + "-key"}
                        value={item.envKey || ''}
-                       onChange={(_, value) => handleEnvValue('envKey', value, idx, onChange, additem, itemCount, item.envValue)} />
+                       validated={validationFailed?.envKey ? "error" : "default"}
+                       onChange={(_event, value) => {
+                           utils.validationClear(validationFailed, "envKey", onValidationChange);
+                           utils.validationDebounce(() => onValidationChange(validateEnvVar(value, "envKey", validationFailed)));
+                           handleEnvValue('envKey', value, idx, onChange, additem, itemCount, item.envValue);
+                       }} />
+                <FormHelper helperTextInvalid={validationFailed?.envKey} />
             </FormGroup>
-            <FormGroup className="pf-m-6-col-on-md" label={_("Value")} fieldId={id + "-value-address"}>
+            <FormGroup className="pf-m-6-col-on-md"
+                id={id + "-value-group"}
+                label={_("Value")}
+                fieldId={id + "-value-address"}
+                isRequired
+            >
                 <TextInput id={id + "-value"}
                        value={item.envValue || ''}
-                       onChange={(_, value) => handleEnvValue('envValue', value, idx, onChange, additem, itemCount, item.envKey)} />
+                       validated={validationFailed?.envValue ? "error" : "default"}
+                       onChange={(_event, value) => {
+                           utils.validationClear(validationFailed, "envValue", onValidationChange);
+                           utils.validationDebounce(() => onValidationChange(validateEnvVar(value, "envValue", validationFailed)));
+                           handleEnvValue('envValue', value, idx, onChange, additem, itemCount, item.envValue);
+                       }} />
+                <FormHelper helperTextInvalid={validationFailed?.envValue} />
             </FormGroup>
             <FormGroup className="pf-m-1-col-on-md remove-button-group">
                 <Button variant='plain'
diff --git a/src/ImageRunModal.jsx b/src/ImageRunModal.jsx
index 1df83293f..b875db75c 100644
--- a/src/ImageRunModal.jsx
+++ b/src/ImageRunModal.jsx
@@ -27,7 +27,7 @@ import { onDownloadContainer, onDownloadContainerFinished } from './Containers.j
 import { PublishPort, validatePublishPort } from './PublishPort.jsx';
 import { DynamicListForm } from 'DynamicListForm.jsx';
 import { validateVolume, Volume } from './Volume.jsx';
-import { EnvVar } from './Env.jsx';
+import { EnvVar, validateEnvVar } from './Env.jsx';
 
 import { debounce } from 'throttle-debounce';
 
@@ -592,7 +592,7 @@ export class ImageRunModal extends React.Component {
     };
 
     validateForm = () => {
-        const { publish, volumes } = this.state;
+        const { publish, volumes, env } = this.state;
         const validationFailed = { };
 
         const publishValidation = publish.map(a => {
@@ -608,14 +608,24 @@ export class ImageRunModal extends React.Component {
 
         const volumesValidation = volumes.map(a => {
             let volumesValidation;
-            volumesValidation = validateVolume(a, "hostPath", volumesValidation);
-            volumesValidation = validateVolume(a, "containerPath", volumesValidation);
+            volumesValidation = validateVolume(a.hostPath, "hostPath", volumesValidation);
+            volumesValidation = validateVolume(a.containerPath, "containerPath", volumesValidation);
 
             return volumesValidation;
         });
         if (volumesValidation.some(entry => Object.keys(entry).length > 0))
             validationFailed.volumes = volumesValidation;
 
+        const envValidation = env.map(a => {
+            let envValidation;
+            envValidation = validateEnvVar(a.envKey, "envKey", envValidation);
+            envValidation = validateEnvVar(a.envValue, "envValue", envValidation);
+
+            return envValidation;
+        });
+        if (envValidation.some(entry => Object.keys(entry).length > 0))
+            validationFailed.env = envValidation;
+
         this.setState({ validationFailed });
 
         return this.isFormValid(validationFailed);
@@ -963,6 +973,8 @@ export class ImageRunModal extends React.Component {
                                  formclass='env-form'
                                  label={_("Environment variables")}
                                  actionLabel={_("Add variable")}
+                                 validationFailed={dialogValues.validationFailed.env}
+                                 onValidationChange={value => this.dynamicListOnValidationChange(value, "env")}
                                  onChange={value => this.onValueChanged('env', value)}
                                  default={{ envKey: null, envValue: null }}
                                  helperText={_("Paste one or more lines of key=value pairs into any field for bulk import")}
diff --git a/test/check-application b/test/check-application
index fb95dfe75..6c3efd1bb 100755
--- a/test/check-application
+++ b/test/check-application
@@ -2303,6 +2303,15 @@ class TestApplication(testlib.MachineCase):
         validateField("#run-image-dialog-volume-0-container-path-group", "", "not be empty", resetValue="/somepath")
         validateField("#run-image-dialog-volume-0-container-path-group", "", "not be empty", tryCreate=True, resetValue="/somepath")
 
+        # Test validation of environment variables
+        b.click('.env-form .btn-add')
+        b.set_input_text("#run-image-dialog-env-0-key-group input", "sometext")
+        b.set_input_text("#run-image-dialog-env-0-value-group input", "sometext")
+        validateField("#run-image-dialog-env-0-key-group", "", "must not be empty", resetValue="sometext")
+        validateField("#run-image-dialog-env-0-key-group", "", "must not be empty", tryCreate=True, resetValue="sometext")
+        validateField("#run-image-dialog-env-0-value-group", "", "must not be empty", resetValue="sometext")
+        validateField("#run-image-dialog-env-0-value-group", "", "must not be empty", tryCreate=True, resetValue="sometext")
+
         b.set_input_text("#run-image-dialog-name", container_name)
 
         # Port address is already in use