[Andruil] Defend for containers UI fixes (#149372)

## Summary

- fix to invalid yaml causing null pointer in general view. 
- also fixed incrementName function to work past two digits when
duplicating selectors
- removed "click here" placeholder in yaml editor help text
- fixed usability issue with removing all conditions in a selector.
previously would be prevented, but now allows and shows a descriptive
error.

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

Co-authored-by: Karl Godard <karlgodard@elastic.co>

x-pack/plugins/cloud_defend/public/components/control_general_view/index.test.tsx

@@ -10,7 +10,7 @@ import { render, waitFor } from '@testing-library/react';
 import { coreMock } from '@kbn/core/public/mocks';
 import userEvent from '@testing-library/user-event';
 import { TestProvider } from '../../test/test_provider';
-import { getCloudDefendNewPolicyMock } from '../../test/mocks';
+import { getCloudDefendNewPolicyMock, MOCK_YAML_INVALID_CONFIGURATION } from '../../test/mocks';
 import { ControlGeneralView } from '.';
 import { getInputFromPolicy } from '../../common/utils';
 import { INPUT_CONTROL } from '../../../common/constants';
@@ -106,4 +106,13 @@ describe('<ControlGeneralView />', () => {
 
     expect(getByTitle('Remove excludeCustomNginxBuild3 from selection in this group')).toBeTruthy();
   });
+
+  it('doesnt blow up if invalid yaml passed in', async () => {
+    const { queryAllByTestId } = render(
+      <WrappedComponent policy={getCloudDefendNewPolicyMock(MOCK_YAML_INVALID_CONFIGURATION)} />
+    );
+
+    expect(queryAllByTestId('cloud-defend-selector')).toHaveLength(0);
+    expect(queryAllByTestId('cloud-defend-response')).toHaveLength(0);
+  });
 });

x-pack/plugins/cloud_defend/public/components/control_general_view/index.tsx

@@ -42,7 +42,7 @@ export const ControlGeneralView = ({ policy, onChange, show }: ViewDeps) => {
     try {
       const result = yaml.load(configuration);
 
-      if (result) {
+      if (result && result.hasOwnProperty('selectors') && result.hasOwnProperty('responses')) {
         return result;
       }
     } catch {
@@ -77,8 +77,11 @@ export const ControlGeneralView = ({ policy, onChange, show }: ViewDeps) => {
   const incrementName = useCallback(
     (name: string): string => {
       // increment name using ints
-      const lastChar = parseInt(name.slice(-1), 10);
-      const newName = isNaN(lastChar) ? name + '1' : name.slice(0, -1) + (lastChar + 1);
+      const numberSuffix = name.search(/\d+$/);
+      const newName =
+        numberSuffix !== -1
+          ? name.slice(0, numberSuffix) + (parseInt(name.slice(numberSuffix), 10) + 1)
+          : name + '1';
       const dupe = selectors.find((selector) => selector.name === newName);
 
       if (dupe) {

x-pack/plugins/cloud_defend/public/components/control_general_view/translations.ts

@@ -76,6 +76,10 @@ export const name = i18n.translate('xpack.cloudDefend.name', {
   defaultMessage: 'Name',
 });
 
+export const errorConditionRequired = i18n.translate('xpack.cloudDefend.errorConditionRequired', {
+  defaultMessage: 'At least one condition per selector is required.',
+});
+
 export const errorDuplicateName = i18n.translate('xpack.cloudDefend.errorDuplicateName', {
   defaultMessage: 'This name is already used by another selector.',
 });

x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx

@@ -118,21 +118,33 @@ describe('<ControlGeneralViewSelector />', () => {
     expect(updatedOptions[0]).not.toHaveTextContent('containerImageName');
   });
 
-  it('ensures at least one condition is provided, and a value specified', async () => {
-    const { getByText, getByTestId } = render(<WrappedComponent />);
+  it('shows an error if no conditions are added', async () => {
+    const { getByText, getByTestId, rerender } = render(<WrappedComponent />);
 
     userEvent.click(getByTestId('cloud-defend-btnremovecondition-operation'));
 
-    expect(onChange.mock.calls).toHaveLength(0); // because operation is the only condition, it should not have been removed.
+    const updatedSelector: ControlSelector = { ...onChange.mock.calls[0][0] };
 
+    rerender(<WrappedComponent selector={updatedSelector} />);
+
+    await waitFor(() => expect(getByText(i18n.errorConditionRequired)).toBeTruthy());
+
+    expect(onChange.mock.calls[0][0]).toHaveProperty('hasErrors');
+  });
+
+  it('shows an error if no values provided for condition', async () => {
+    const { getByText, getByTestId } = render(<WrappedComponent />);
     const addConditionBtn = getByTestId('cloud-defend-btnaddselectorcondition');
+
+    userEvent.click(getByTestId('cloud-defend-btnremovecondition-operation'));
     userEvent.click(addConditionBtn);
 
     await waitFor(() => userEvent.click(getByText('Container image name'))); // add containerImageName
 
-    expect(onChange.mock.calls).toHaveLength(1);
-    expect(onChange.mock.calls[0][0]).toHaveProperty('containerImageName');
-    expect(onChange.mock.calls[0][0]).toHaveProperty('hasErrors');
+    expect(onChange.mock.calls).toHaveLength(2);
+    expect(onChange.mock.calls[1][0]).toHaveProperty('containerImageName');
+    expect(onChange.mock.calls[1][0]).toHaveProperty('hasErrors');
+    expect(getByText(i18n.errorValueRequired)).toBeTruthy();
   });
 
   it('prevents conditions from having values that exceed MAX_CONDITION_VALUE_LENGTH_BYTES', async () => {

x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.tsx

@@ -64,6 +64,14 @@ export const ControlGeneralViewSelector = ({
     setAddConditionOpen(false);
   }, []);
 
+  const remainingProps = useMemo(() => {
+    return Object.keys(ControlSelectorCondition).filter(
+      (condition) => !selector.hasOwnProperty(condition)
+    );
+  }, [selector]);
+
+  const conditionsAdded = Object.keys(ControlSelectorCondition).length - remainingProps.length;
+
   const onRemoveClicked = useCallback(() => {
     // we prevent the removal of the last selector to avoid an empty state
     if (selectors.length > 1) {
@@ -106,11 +114,11 @@ export const ControlGeneralViewSelector = ({
       const updatedSelector = { ...selector };
 
       updatedSelector.name = value;
-      updatedSelector.hasErrors = Object.keys(errorMap).length > 0;
+      updatedSelector.hasErrors = Object.keys(errorMap).length > 0 || conditionsAdded === 0;
 
       onChange(updatedSelector, index);
     },
-    [errorMap, index, onChange, selector, selectors]
+    [errorMap, index, conditionsAdded, onChange, selector, selectors]
   );
 
   const onChangeCondition = useCallback(
@@ -140,12 +148,12 @@ export const ControlGeneralViewSelector = ({
         delete errorMap[prop];
       }
 
-      updatedSelector.hasErrors = Object.keys(errorMap).length > 0;
+      updatedSelector.hasErrors = Object.keys(errorMap).length > 0 || conditionsAdded === 0;
       setErrorMap({ ...errorMap });
 
       onChange(updatedSelector, index);
     },
-    [errorMap, index, onChange, selector]
+    [errorMap, index, conditionsAdded, onChange, selector]
   );
 
   const onAddCondition = useCallback(
@@ -163,12 +171,12 @@ export const ControlGeneralViewSelector = ({
 
       delete errorMap[prop];
       setErrorMap({ ...errorMap });
-      updatedSelector.hasErrors = Object.keys(errorMap).length > 0;
+      updatedSelector.hasErrors = Object.keys(errorMap).length > 0 || conditionsAdded === 1;
 
       onChange(updatedSelector, index);
       closeAddCondition();
     },
-    [closeAddCondition, errorMap, index, onChange, selector]
+    [closeAddCondition, conditionsAdded, errorMap, index, onChange, selector]
   );
 
   const onAddValueToCondition = useCallback(
@@ -184,18 +192,16 @@ export const ControlGeneralViewSelector = ({
   );
 
   const errors = useMemo(() => {
-    return Object.keys(errorMap).reduce<string[]>((prev, current) => {
+    const errs = Object.keys(errorMap).reduce<string[]>((prev, current) => {
       return prev.concat(errorMap[current]);
     }, []);
-  }, [errorMap]);
 
-  const remainingProps = useMemo(() => {
-    return Object.keys(ControlSelectorCondition).filter(
-      (condition) => !selector.hasOwnProperty(condition)
-    );
-  }, [selector]);
+    if (conditionsAdded === 0) {
+      errs.push(i18n.errorConditionRequired);
+    }
 
-  const conditionsAdded = Object.keys(ControlSelectorCondition).length - remainingProps.length;
+    return errs;
+  }, [errorMap, conditionsAdded]);
 
   return (
     <EuiAccordion
@@ -305,7 +311,6 @@ export const ControlGeneralViewSelector = ({
                   </EuiFlexItem>
                   <EuiFlexItem grow={false}>
                     <EuiButtonIcon
-                      disabled={conditionsAdded < 2}
                       iconType="cross"
                       onClick={() => onRemoveCondition(prop)}
                       aria-label="Remove condition"

x-pack/plugins/cloud_defend/public/components/control_settings/index.tsx

@@ -29,7 +29,7 @@ export const ControlSettings = ({ policy, onChange }: SettingsDeps) => {
   const isGeneralViewSelected = viewMode === VIEW_MODE_GENERAL;
   const isYamlViewSelected = viewMode === VIEW_MODE_YAML;
 
-  const onChanges = useCallback(
+  const onGeneralChanges = useCallback(
     (opts: OnChangeDeps) => {
       opts.updatedPolicy = policy;
       onChange(opts);
@@ -38,6 +38,17 @@ export const ControlSettings = ({ policy, onChange }: SettingsDeps) => {
     [onChange, policy]
   );
 
+  const onYamlChanges = useCallback(
+    (opts: OnChangeDeps) => {
+      if (isYamlViewSelected) {
+        opts.updatedPolicy = policy;
+        onChange(opts);
+        setIsValid(opts.isValid);
+      }
+    },
+    [isYamlViewSelected, onChange, policy]
+  );
+
   return (
     <EuiFlexGroup direction="column">
       <EuiFlexItem>
@@ -63,8 +74,17 @@ export const ControlSettings = ({ policy, onChange }: SettingsDeps) => {
         </EuiTabs>
       </EuiFlexItem>
       <EuiFlexItem>
-        <ControlGeneralView show={isGeneralViewSelected} policy={policy} onChange={onChanges} />
-        <ControlYamlView show={isYamlViewSelected} policy={policy} onChange={onChanges} />
+        {/** general view removed from DOM for performance and to avoid errors when invalid yaml is passed to it**/}
+        {isGeneralViewSelected && (
+          <ControlGeneralView
+            show={isGeneralViewSelected}
+            policy={policy}
+            onChange={onGeneralChanges}
+          />
+        )}
+        {/** Yaml view is kept in the dom at all times to prevent some sizing/rendering issues.
+        Also only listening for changes if yaml view visible to avoid isValid race condition **/}
+        <ControlYamlView show={isYamlViewSelected} policy={policy} onChange={onYamlChanges} />
       </EuiFlexItem>
     </EuiFlexGroup>
   );

x-pack/plugins/cloud_defend/public/components/control_yaml_view/translations.ts

@@ -13,6 +13,5 @@ export const errorAlertActionRequired = i18n.translate('xpack.cloudDefend.alertA
 });
 
 export const controlYamlHelp = i18n.translate('xpack.cloudDefend.controlYamlHelp', {
-  defaultMessage:
-    'Configure BPF/LSM controls by creating selectors, and responses below. To learn more click <here>',
+  defaultMessage: 'Configure BPF/LSM controls by creating selectors, and responses below.',
 });

x-pack/plugins/cloud_defend/public/test/mocks.ts

@@ -38,8 +38,7 @@ responses:
 `;
 
 export const MOCK_YAML_INVALID_CONFIGURATION = `
-selectrs:
-reeesponses:
+s
 `;
 
 export const getCloudDefendNewPolicyMock = (yaml = MOCK_YAML_CONFIGURATION): NewPackagePolicy => ({