Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Improve rules exception flyout opening for the indices with huge amount of fields #159216

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
6736b1e
[Security Solution] Improve rules exception flyout opening for the in…
e40pud Jun 6, 2023
28c1d32
Remove obsolete includeUnmapped use
e40pud Jun 7, 2023
b9fa50a
Merge branch 'main' into security/bugfix/158751-improved-exception-fi…
kibanamachine Jun 7, 2023
538460a
Merge branch 'main' into security/bugfix/158751-improved-exception-fi…
kibanamachine Jun 9, 2023
742824b
Merge branch 'main' into security/bugfix/158751-improved-exception-fi…
kibanamachine Jun 12, 2023
77cf06d
Merge branch 'main' into security/bugfix/158751-improved-exception-fi…
kibanamachine Jun 13, 2023
af2f91a
Review feedback
e40pud Jun 13, 2023
e35dff3
Review feedback
e40pud Jun 14, 2023
4083789
Merge branch 'main' into security/bugfix/158751-improved-exception-fi…
kibanamachine Jun 14, 2023
42da19b
Merge branch 'main' into security/bugfix/158751-improved-exception-fi…
kibanamachine Jun 14, 2023
c077b2b
Merge branch 'main' into security/bugfix/158751-improved-exception-fi…
kibanamachine Jun 14, 2023
fc6e19d
Merge branch 'main' into security/bugfix/158751-improved-exception-fi…
kibanamachine Jun 15, 2023
72ca31a
Merge branch 'main' into security/bugfix/158751-improved-exception-fi…
kibanamachine Jun 15, 2023
ef9cb0f
Merge branch 'main' into security/bugfix/158751-improved-exception-fi…
kibanamachine Jun 15, 2023
e0a457d
Merge branch 'main' into security/bugfix/158751-improved-exception-fi…
kibanamachine Jun 15, 2023
57f6415
Merge branch 'main' into security/bugfix/158751-improved-exception-fi…
kibanamachine Jun 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -197,14 +197,15 @@ describe('BuilderEntryItem', () => {
);
});

test('it render mapping issues warning text when field has mapping conflicts', () => {
test('it render mapping issues warning text when field has mapping conflicts', async () => {
const field = getField('mapping issues');
wrapper = mount(
<BuilderEntryItem
autocompleteService={autocompleteStartMock}
entry={{
correspondingKeywordField: undefined,
entryIndex: 0,
field: getField('mapping issues'),
field,
id: '123',
nested: undefined,
operator: isOperator,
Expand All @@ -223,12 +224,16 @@ describe('BuilderEntryItem', () => {
setWarningsExist={jest.fn()}
showLabel
allowCustomOptions
getExtendedFields={(): Promise<FieldSpec[]> => Promise.resolve([field])}
/>
);

expect(wrapper.find('.euiFormHelpText.euiFormRow__text').text()).toMatch(
/This field is defined as different types across the following indices or is unmapped. This can cause unexpected query results./
);
await waitFor(() => {
wrapper.update();
expect(wrapper.find('.euiFormHelpText.euiFormRow__text').text()).toMatch(
/This field is defined as different types across the following indices or is unmapped. This can cause unexpected query results./
);
});
});

test('it renders field values correctly when operator is "isOperator"', () => {
Expand Down
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React, { useCallback, useMemo } from 'react';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { FormattedMessage } from '@kbn/i18n-react';
import {
EuiAccordion,
Expand All @@ -26,6 +26,7 @@ import {
} from '@kbn/securitysolution-io-ts-list-types';
import {
BuilderEntry,
DataViewField,
EXCEPTION_OPERATORS_ONLY_LISTS,
FormattedBuilderEntry,
OperatorOption,
Expand Down Expand Up @@ -87,6 +88,7 @@ export interface EntryItemProps {
isDisabled?: boolean;
operatorsList?: OperatorOption[];
allowCustomOptions?: boolean;
getExtendedFields?: (fields: string[]) => Promise<DataViewField[]>;
}

export const BuilderEntryItem: React.FC<EntryItemProps> = ({
Expand All @@ -106,6 +108,7 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
isDisabled = false,
operatorsList,
allowCustomOptions = false,
getExtendedFields,
}): JSX.Element => {
const sPaddingSize = useEuiPaddingSize('s');

Expand Down Expand Up @@ -175,6 +178,22 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
[onChange, entry]
);

const [extendedField, setExtendedField] = useState<DataViewField | null>(null);
useEffect(() => {
if (!entry.field?.name) {
setExtendedField(null);
}
const fetchExtendedField = async (): Promise<void> => {
const fieldName = entry.field?.name;
if (getExtendedFields && fieldName) {
const extendedFields = await getExtendedFields([fieldName]);
const field = extendedFields.find((f) => f.name === fieldName) ?? null;
setExtendedField(field);
}
};
fetchExtendedField();
}, [entry.field?.name, getExtendedFields]);

const isFieldComponentDisabled = useMemo(
(): boolean =>
isDisabled ||
Expand Down Expand Up @@ -212,8 +231,11 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
);

const warningIconCss = { marginRight: `${sPaddingSize}` };
const getMappingConflictsWarning = (field: DataViewFieldBase): React.ReactNode | null => {
const conflictsInfo = getMappingConflictsInfo(field);
const getMappingConflictsWarning = (): React.ReactNode | null => {
if (!extendedField) {
return null;
}
const conflictsInfo = getMappingConflictsInfo(extendedField);
if (!conflictsInfo) {
return null;
}
Expand All @@ -238,13 +260,13 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
data-test-subj="mappingConflictsAccordion"
>
<div data-test-subj="mappingConflictsDescription">
{conflictsInfo.map((info) => {
{conflictsInfo.map((info, idx) => {
const groupDetails = info.groupedIndices.map(
({ name, count }) =>
`${count > 1 ? i18n.CONFLICT_MULTIPLE_INDEX_DESCRIPTION(name, count) : name}`
);
return (
<>
<EuiFlexItem key={`${idx}`}>
<EuiSpacer size="s" />
{`${
info.totalIndexCount > 1
Expand All @@ -254,7 +276,7 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
)
: info.type
}: ${groupDetails.join(', ')}`}
</>
</EuiFlexItem>
);
})}
<EuiSpacer size="s" />
Expand All @@ -268,12 +290,12 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
entry.nested == null && allowCustomOptions ? i18n.CUSTOM_COMBOBOX_OPTION_TEXT : undefined;

const helpText =
entry.field?.conflictDescriptions == null ? (
extendedField?.conflictDescriptions == null ? (
customOptionText
) : (
<>
{customOptionText}
{getMappingConflictsWarning(entry.field)}
{getMappingConflictsWarning()}
</>
);
return (
Expand All @@ -295,8 +317,9 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
osTypes,
isDisabled,
handleFieldChange,
sPaddingSize,
allowCustomOptions,
sPaddingSize,
extendedField,
]
);

Expand Down
Expand Up @@ -13,6 +13,7 @@ import { HttpStart } from '@kbn/core/public';
import { ExceptionListType, OsTypeArray } from '@kbn/securitysolution-io-ts-list-types';
import {
BuilderEntry,
DataViewField,
ExceptionsBuilderExceptionItem,
FormattedBuilderEntry,
OperatorOption,
Expand Down Expand Up @@ -65,6 +66,7 @@ interface BuilderExceptionListItemProps {
isDisabled?: boolean;
operatorsList?: OperatorOption[];
allowCustomOptions?: boolean;
getExtendedFields?: (fields: string[]) => Promise<DataViewField[]>;
}

export const BuilderExceptionListItemComponent = React.memo<BuilderExceptionListItemProps>(
Expand All @@ -88,6 +90,7 @@ export const BuilderExceptionListItemComponent = React.memo<BuilderExceptionList
isDisabled = false,
operatorsList,
allowCustomOptions = false,
getExtendedFields,
}) => {
const handleEntryChange = useCallback(
(entry: BuilderEntry, entryIndex: number): void => {
Expand Down Expand Up @@ -161,6 +164,7 @@ export const BuilderExceptionListItemComponent = React.memo<BuilderExceptionList
}
operatorsList={operatorsList}
allowCustomOptions={allowCustomOptions}
getExtendedFields={getExtendedFields}
/>
</MyOverflowContainer>
<BuilderEntryDeleteButtonComponent
Expand Down
Expand Up @@ -22,6 +22,7 @@ import {
} from '@kbn/securitysolution-io-ts-list-types';
import {
CreateExceptionListItemBuilderSchema,
DataViewField,
ExceptionsBuilderExceptionItem,
ExceptionsBuilderReturnExceptionItem,
OperatorOption,
Expand Down Expand Up @@ -98,6 +99,7 @@ export interface ExceptionBuilderProps {
operatorsList?: OperatorOption[];
exceptionItemName?: string;
allowCustomFieldOptions?: boolean;
getExtendedFields?: (fields: string[]) => Promise<DataViewField[]>;
}

export const ExceptionBuilderComponent = ({
Expand All @@ -121,6 +123,7 @@ export const ExceptionBuilderComponent = ({
osTypes,
operatorsList,
allowCustomFieldOptions = false,
getExtendedFields,
}: ExceptionBuilderProps): JSX.Element => {
const [state, dispatch] = useReducer(exceptionsBuilderReducer(), {
...initialState,
Expand Down Expand Up @@ -442,6 +445,7 @@ export const ExceptionBuilderComponent = ({
isDisabled={isDisabled}
operatorsList={operatorsList}
allowCustomOptions={allowCustomFieldOptions}
getExtendedFields={getExtendedFields}
/>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Expand Up @@ -42,11 +42,7 @@ export const getAllFieldsByName = (
keyBy('name', getAllBrowserFields(browserFields));

export const getIndexFields = memoizeOne(
(
title: string,
fields: IIndexPatternFieldList,
_includeUnmapped: boolean = false
): DataViewBase =>
(title: string, fields: IIndexPatternFieldList): DataViewBase =>
fields && fields.length > 0
? {
fields: fields.map((field) =>
Expand All @@ -66,10 +62,7 @@ export const getIndexFields = memoizeOne(
title,
}
: { fields: [], title },
(newArgs, lastArgs) =>
newArgs[0] === lastArgs[0] &&
newArgs[1].length === lastArgs[1].length &&
newArgs[2] === lastArgs[2]
(newArgs, lastArgs) => newArgs[0] === lastArgs[0] && newArgs[1].length === lastArgs[1].length
);

const DEFAULT_BROWSER_FIELDS = {};
Expand All @@ -96,8 +89,7 @@ interface FetchIndexReturn {
export const useFetchIndex = (
indexNames: string[],
onlyCheckIfIndicesExist: boolean = false,
strategy: 'indexFields' | 'dataView' | typeof ENDPOINT_FIELDS_SEARCH_STRATEGY = 'indexFields',
includeUnmapped: boolean = false
strategy: 'indexFields' | 'dataView' | typeof ENDPOINT_FIELDS_SEARCH_STRATEGY = 'indexFields'
): [boolean, FetchIndexReturn] => {
const { data } = useKibana().services;
const abortCtrl = useRef(new AbortController());
Expand All @@ -121,11 +113,7 @@ export const useFetchIndex = (
abortCtrl.current = new AbortController();
const dv = await data.dataViews.create({ title: iNames.join(','), allowNoIndex: true });
const dataView = dv.toSpec();
const { browserFields } = getDataViewStateFromIndexFields(
iNames,
dataView.fields,
includeUnmapped
);
const { browserFields } = getDataViewStateFromIndexFields(iNames, dataView.fields);

previousIndexesName.current = dv.getIndexPattern().split(',');

Expand All @@ -135,7 +123,7 @@ export const useFetchIndex = (
browserFields,
indexes: dv.getIndexPattern().split(','),
indexExists: dv.getIndexPattern().split(',').length > 0,
indexPatterns: getIndexFields(dv.getIndexPattern(), dv.fields, includeUnmapped),
indexPatterns: getIndexFields(dv.getIndexPattern(), dv.fields),
});
} catch (exc) {
setState({
Expand All @@ -152,7 +140,7 @@ export const useFetchIndex = (

asyncSearch();
},
[addError, data.dataViews, includeUnmapped, indexNames, state]
[addError, data.dataViews, indexNames, state]
);

useEffect(() => {
Expand Down
Expand Up @@ -48,11 +48,7 @@ interface DataViewInfo {
* VERY mutatious on purpose to improve the performance of the transform.
*/
export const getDataViewStateFromIndexFields = memoizeOne(
(
_title: string,
fields: DataViewSpec['fields'],
_includeUnmapped: boolean = false
): DataViewInfo => {
(_title: string, fields: DataViewSpec['fields']): DataViewInfo => {
// Adds two dangerous casts to allow for mutations within this function
type DangerCastForMutation = Record<string, {}>;
if (fields == null) {
Expand All @@ -72,10 +68,7 @@ export const getDataViewStateFromIndexFields = memoizeOne(
return { browserFields: browserFields as DangerCastForBrowserFieldsMutation };
}
},
(newArgs, lastArgs) =>
newArgs[0] === lastArgs[0] &&
newArgs[1]?.length === lastArgs[1]?.length &&
newArgs[2] === lastArgs[2]
(newArgs, lastArgs) => newArgs[0] === lastArgs[0] && newArgs[1]?.length === lastArgs[1]?.length
);

export const useDataView = (): {
Expand Down
Expand Up @@ -438,8 +438,7 @@ export const useSourcererDataView = (
const browserFields = useCallback(() => {
const { browserFields: dataViewBrowserFields } = getDataViewStateFromIndexFields(
sourcererDataView.patternList.join(','),
sourcererDataView.fields,
false
sourcererDataView.fields
);
return dataViewBrowserFields;
}, [sourcererDataView.fields, sourcererDataView.patternList]);
Expand Down
Expand Up @@ -85,6 +85,7 @@ describe('When the add exception modal is opened', () => {
mockFetchIndexPatterns.mockImplementation(() => ({
isLoading: false,
indexPatterns: stubIndexPattern,
getExtendedFields: () => Promise.resolve([]),
}));

mockUseSignalIndex.mockImplementation(() => ({
Expand Down Expand Up @@ -153,6 +154,7 @@ describe('When the add exception modal is opened', () => {
mockFetchIndexPatterns.mockImplementation(() => ({
isLoading: true,
indexPatterns: { fields: [], title: 'foo' },
getExtendedFields: () => Promise.resolve([]),
}));

wrapper = mount(
Expand Down
Expand Up @@ -114,7 +114,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
onCancel,
onConfirm,
}: AddExceptionFlyoutProps) {
const { isLoading, indexPatterns } = useFetchIndexPatterns(rules);
const { isLoading, indexPatterns, getExtendedFields } = useFetchIndexPatterns(rules);
const [isSubmitting, submitNewExceptionItems] = useAddNewExceptionItems();
const [isClosingAlerts, closeAlerts] = useCloseAlertsFromExceptions();
const invalidateFetchRuleByIdQuery = useInvalidateFetchRuleByIdQuery();
Expand Down Expand Up @@ -501,6 +501,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
onExceptionItemAdd={setExceptionItemsToAdd}
onSetErrorExists={setConditionsValidationError}
onFilterIndexPatterns={filterIndexPatterns}
getExtendedFields={getExtendedFields}
/>

{listType !== ExceptionListTypeEnum.ENDPOINT && !sharedListToAddTo?.length && (
Expand Down
Expand Up @@ -124,6 +124,7 @@ describe('When the edit exception modal is opened', () => {
mockFetchIndexPatterns.mockImplementation(() => ({
isLoading: false,
indexPatterns: stubIndexPattern,
getExtendedFields: () => Promise.resolve([]),
}));
mockUseFindExceptionListReferences.mockImplementation(() => [
false,
Expand Down Expand Up @@ -168,6 +169,7 @@ describe('When the edit exception modal is opened', () => {
mockFetchIndexPatterns.mockImplementation(() => ({
isLoading: true,
indexPatterns: { fields: [], title: 'foo' },
getExtendedFields: () => Promise.resolve([]),
}));

const wrapper = mount(
Expand Down
Expand Up @@ -109,7 +109,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
const rules = useMemo(() => (rule != null ? [rule] : null), [rule]);
const listType = useMemo((): ExceptionListTypeEnum => list.type as ExceptionListTypeEnum, [list]);

const { isLoading, indexPatterns } = useFetchIndexPatterns(rules);
const { isLoading, indexPatterns, getExtendedFields } = useFetchIndexPatterns(rules);
const [isSubmitting, submitEditExceptionItems] = useEditExceptionItems();
const [isClosingAlerts, closeAlerts] = useCloseAlertsFromExceptions();

Expand Down Expand Up @@ -370,6 +370,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
onExceptionItemAdd={setExceptionItemsToAdd}
onSetErrorExists={setConditionsValidationError}
onFilterIndexPatterns={filterIndexPatterns}
getExtendedFields={getExtendedFields}
/>
{!openedFromListDetailPage && listType === ExceptionListTypeEnum.DETECTION && (
<>
Expand Down