Skip to content

Commit

Permalink
[Fleet] Improve space selector validation when not providing valid sp…
Browse files Browse the repository at this point in the history
  • Loading branch information
nchaulet authored Oct 22, 2024
1 parent c9637cf commit b668544
Show file tree
Hide file tree
Showing 6 changed files with 176 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ import { UninstallCommandFlyout } from '../../../../../../components';
import type { ValidationResults } from '../agent_policy_validation';

import { ExperimentalFeaturesService } from '../../../../services';

import { useAgentPolicyFormContext } from '../agent_policy_form';
import { policyHasEndpointSecurity as hasElasticDefend } from '../../../../../../../common/services';

import {
Expand Down Expand Up @@ -127,6 +127,8 @@ export const AgentPolicyAdvancedOptionsContent: React.FunctionComponent<Props> =
const isManagedorAgentlessPolicy =
agentPolicy.is_managed === true || agentPolicy?.supports_agentless === true;

const agentPolicyFormContect = useAgentPolicyFormContext();

const AgentTamperProtectionSectionContent = useMemo(
() => (
<EuiDescribedFormGroup
Expand Down Expand Up @@ -325,32 +327,23 @@ export const AgentPolicyAdvancedOptionsContent: React.FunctionComponent<Props> =
/>
}
>
<EuiFormRow
fullWidth
key="space"
error={
touchedFields.description && validation.description ? validation.description : null
}
<SpaceSelector
isDisabled={disabled}
isInvalid={Boolean(touchedFields.description && validation.description)}
>
<SpaceSelector
isDisabled={disabled}
value={
'space_ids' in agentPolicy && agentPolicy.space_ids
? agentPolicy.space_ids
: [spaceId || 'default']
value={
'space_ids' in agentPolicy && agentPolicy.space_ids
? agentPolicy.space_ids
: [spaceId || 'default']
}
setInvalidSpaceError={agentPolicyFormContect?.setInvalidSpaceError}
onChange={(newValue) => {
if (newValue.length === 0) {
return;
}
onChange={(newValue) => {
if (newValue.length === 0) {
return;
}
updateAgentPolicy({
space_ids: newValue,
});
}}
/>
</EuiFormRow>
updateAgentPolicy({
space_ids: newValue,
});
}}
/>
</EuiDescribedFormGroup>
) : null}
<EuiDescribedFormGroup
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';
import { fireEvent } from '@testing-library/react';

import { useAgentPoliciesSpaces } from '../../../../../../hooks/use_request/spaces';

import { createFleetTestRendererMock } from '../../../../../../mock';

import { SpaceSelector } from './space_selector';

jest.mock('../../../../../../hooks/use_request/spaces');

describe('Space Selector', () => {
beforeEach(() => {
jest.mocked(useAgentPoliciesSpaces).mockReturnValue({
data: {
items: [
{
name: 'Default',
id: 'default',
},
{
name: 'Test',
id: 'test',
},
],
},
} as any);
});
function render() {
const renderer = createFleetTestRendererMock();
const onChange = jest.fn();
const setInvalidSpaceError = jest.fn();
const result = renderer.render(
<SpaceSelector setInvalidSpaceError={setInvalidSpaceError} onChange={onChange} value={[]} />
);

return {
result,
onChange,
setInvalidSpaceError,
};
}

it('should render invalid space errors', () => {
const { result, onChange, setInvalidSpaceError } = render();
const inputEl = result.getByTestId('comboBoxSearchInput');
fireEvent.change(inputEl, {
target: { value: 'invalidSpace' },
});
fireEvent.keyDown(inputEl, { key: 'Enter', code: 'Enter' });
expect(result.container).toHaveTextContent('invalidSpace is not a valid space.');
expect(onChange).not.toBeCalled();
expect(setInvalidSpaceError).toBeCalledWith(true);
});

it('should clear invalid space errors', () => {
const { result, setInvalidSpaceError } = render();
const inputEl = result.getByTestId('comboBoxSearchInput');
fireEvent.change(inputEl, {
target: { value: 'invalidSpace' },
});
fireEvent.keyDown(inputEl, { key: 'Enter', code: 'Enter' });
expect(result.container).toHaveTextContent('invalidSpace is not a valid space.');
fireEvent.change(inputEl, {
target: { value: '' },
});
fireEvent.keyDown(inputEl, { key: 'Enter', code: 'Enter' });
expect(result.container).not.toHaveTextContent('invalidSpace is not a valid space.');
expect(setInvalidSpaceError).toBeCalledWith(false);
});

it('should accept valid space', () => {
const { result, onChange, setInvalidSpaceError } = render();
const inputEl = result.getByTestId('comboBoxSearchInput');
fireEvent.change(inputEl, {
target: { value: 'test' },
});
fireEvent.keyDown(inputEl, { key: 'Enter', code: 'Enter' });
expect(result.container).not.toHaveTextContent('test is not a valid space.');
expect(onChange).toBeCalledWith(['test']);
expect(setInvalidSpaceError).not.toBeCalledWith(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { type EuiComboBoxOptionOption, EuiHealth } from '@elastic/eui';
import { type EuiComboBoxOptionOption, EuiHealth, EuiFormRow } from '@elastic/eui';
import { EuiComboBox } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React, { useMemo } from 'react';
Expand All @@ -16,11 +16,19 @@ export interface SpaceSelectorProps {
value: string[];
onChange: (newVal: string[]) => void;
isDisabled?: boolean;
setInvalidSpaceError?: (hasError: boolean) => void;
}

export const SpaceSelector: React.FC<SpaceSelectorProps> = ({ value, onChange, isDisabled }) => {
export const SpaceSelector: React.FC<SpaceSelectorProps> = ({
setInvalidSpaceError,
value,
onChange,
isDisabled,
}) => {
const res = useAgentPoliciesSpaces();

const [error, setError] = React.useState<string>();

const renderOption = React.useCallback(
(option: any, searchValue: string, contentClassName: string) => (
<EuiHealth color={option.color}>
Expand Down Expand Up @@ -57,20 +65,41 @@ export const SpaceSelector: React.FC<SpaceSelectorProps> = ({ value, onChange, i
}, [options, value, res.isInitialLoading]);

return (
<EuiComboBox
data-test-subj={'spaceSelectorComboBox'}
aria-label={i18n.translate('xpack.fleet.agentPolicies.spaceSelectorLabel', {
defaultMessage: 'Spaces',
})}
<EuiFormRow
fullWidth
options={options}
renderOption={renderOption}
selectedOptions={selectedOptions}
isDisabled={res.isInitialLoading || isDisabled}
isClearable={false}
onChange={(newOptions) => {
onChange(newOptions.map(({ key }) => key as string));
}}
/>
key="space"
error={error}
isDisabled={isDisabled}
isInvalid={Boolean(error)}
>
<EuiComboBox
data-test-subj={'spaceSelectorComboBox'}
aria-label={i18n.translate('xpack.fleet.agentPolicies.spaceSelectorLabel', {
defaultMessage: 'Spaces',
})}
fullWidth
options={options}
renderOption={renderOption}
selectedOptions={selectedOptions}
isDisabled={res.isInitialLoading || isDisabled}
isClearable={false}
onSearchChange={(searchValue, hasMatchingOptions) => {
const newError =
searchValue.length === 0 || hasMatchingOptions
? undefined
: i18n.translate('xpack.fleet.agentPolicies.spaceSelectorInvalid', {
defaultMessage: '{space} is not a valid space.',
values: { space: searchValue },
});
setError(newError);
if (setInvalidSpaceError) {
setInvalidSpaceError(!!newError);
}
}}
onChange={(newOptions) => {
onChange(newOptions.map(({ key }) => key as string));
}}
/>
</EuiFormRow>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ interface Props {
isEditing?: boolean;
// form error state is passed up to the form
updateAdvancedSettingsHasErrors: (hasErrors: boolean) => void;
setInvalidSpaceError: (hasErrors: boolean) => void;
}
const AgentPolicyFormContext = React.createContext<
| {
agentPolicy: Partial<NewAgentPolicy | AgentPolicy> & { [key: string]: any };
updateAgentPolicy: (u: Partial<NewAgentPolicy | AgentPolicy>) => void;
updateAdvancedSettingsHasErrors: (hasErrors: boolean) => void;
setInvalidSpaceError: (hasErrors: boolean) => void;
}
| undefined
>(undefined);
Expand All @@ -67,6 +69,7 @@ export const AgentPolicyForm: React.FunctionComponent<Props> = ({
validation,
isEditing = false,
updateAdvancedSettingsHasErrors,
setInvalidSpaceError,
}) => {
const authz = useAuthz();
const isDisabled = !authz.fleet.allAgentPolicies;
Expand Down Expand Up @@ -97,7 +100,12 @@ export const AgentPolicyForm: React.FunctionComponent<Props> = ({

return (
<AgentPolicyFormContext.Provider
value={{ agentPolicy, updateAgentPolicy, updateAdvancedSettingsHasErrors }}
value={{
agentPolicy,
updateAgentPolicy,
updateAdvancedSettingsHasErrors,
setInvalidSpaceError,
}}
>
<EuiForm>
{!isEditing ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export const SettingsView = memo<{ agentPolicy: AgentPolicy }>(
allowedNamespacePrefixes: spaceSettings?.allowedNamespacePrefixes,
});
const [hasAdvancedSettingsErrors, setHasAdvancedSettingsErrors] = useState<boolean>(false);
const [hasInvalidSpaceError, setInvalidSpaceError] = useState<boolean>(false);

const updateAgentPolicy = (updatedFields: Partial<AgentPolicy>) => {
setAgentPolicy({
Expand Down Expand Up @@ -183,6 +184,7 @@ export const SettingsView = memo<{ agentPolicy: AgentPolicy }>(
validation={validation}
isEditing={true}
updateAdvancedSettingsHasErrors={setHasAdvancedSettingsErrors}
setInvalidSpaceError={setInvalidSpaceError}
/>

{hasChanges ? (
Expand Down Expand Up @@ -219,7 +221,8 @@ export const SettingsView = memo<{ agentPolicy: AgentPolicy }>(
isDisabled={
isLoading ||
Object.keys(validation).length > 0 ||
hasAdvancedSettingsErrors
hasAdvancedSettingsErrors ||
hasInvalidSpaceError
}
btnProps={{
color: 'text',
Expand All @@ -242,7 +245,8 @@ export const SettingsView = memo<{ agentPolicy: AgentPolicy }>(
!hasAllAgentPoliciesPrivileges ||
isLoading ||
Object.keys(validation).length > 0 ||
hasAdvancedSettingsErrors
hasAdvancedSettingsErrors ||
hasInvalidSpaceError
}
data-test-subj="agentPolicyDetailsSaveButton"
iconType="save"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export const CreateAgentPolicyFlyout: React.FunctionComponent<Props> = ({
allowedNamespacePrefixes: spaceSettings?.allowedNamespacePrefixes,
});
const [hasAdvancedSettingsErrors, setHasAdvancedSettingsErrors] = useState<boolean>(false);
const [hasInvalidSpaceError, setInvalidSpaceError] = useState<boolean>(false);

const updateAgentPolicy = (updatedFields: Partial<NewAgentPolicy>) => {
setAgentPolicy({
Expand Down Expand Up @@ -104,6 +105,7 @@ export const CreateAgentPolicyFlyout: React.FunctionComponent<Props> = ({
updateSysMonitoring={(newValue) => setWithSysMonitoring(newValue)}
validation={validation}
updateAdvancedSettingsHasErrors={setHasAdvancedSettingsErrors}
setInvalidSpaceError={setInvalidSpaceError}
/>
</EuiFlyoutBody>
);
Expand All @@ -130,7 +132,10 @@ export const CreateAgentPolicyFlyout: React.FunctionComponent<Props> = ({
<EuiFlexItem grow={false}>
<DevtoolsRequestFlyoutButton
isDisabled={
isLoading || Object.keys(validation).length > 0 || hasAdvancedSettingsErrors
isLoading ||
Object.keys(validation).length > 0 ||
hasAdvancedSettingsErrors ||
hasInvalidSpaceError
}
description={i18n.translate(
'xpack.fleet.createAgentPolicy.devtoolsRequestDescription',
Expand All @@ -150,7 +155,8 @@ export const CreateAgentPolicyFlyout: React.FunctionComponent<Props> = ({
!hasFleetAllAgentPoliciesPrivileges ||
isLoading ||
Object.keys(validation).length > 0 ||
hasAdvancedSettingsErrors
hasAdvancedSettingsErrors ||
hasInvalidSpaceError
}
onClick={async () => {
setIsLoading(true);
Expand Down

0 comments on commit b668544

Please sign in to comment.