Skip to content

Commit

Permalink
[Search] Fixes EQL search strategy (#83064)
Browse files Browse the repository at this point in the history
* Ensure that data is not lost when parsing EQL responses

The shared search utilities expect that response data exists in the
response's body field. However, in an EQL response this information also
exists as a sibling to the body field, and so we must normalize this data
into the body before we can leverage these utilities with EQL queries.

* Remove unused EQL parameters

These were previously needed to work around an index resolution but, but
this has since been resolved upstream in elasticsearch via
elastic/elasticsearch#63573.

* Allow custom test subj for Preview Histogram to propagate to DOM

Previously, custom preview histograms were passing a data-test-subj prop
to our general histogram, but the general histogram did not know/care
about this prop and it did not become a data property on the underlying
DOM element. While most of our tests leveraged enzyme, they could still
query by this react prop and everything worked as expected.

However, now that we want to exercise this behavior in cypress, we need
something to propagate to the DOM so that we can determine which
histogram has rendered, so the prop has been updated to be
`dataTestSubj`, which then becomes a data-test-subj on the histogram's
panel. Tests have been updated accordingly.

* Exercise Query Preview during EQL rule creation

* Asserts that the preview displays a histogram
* Asserts that no error toast is displayed

* Add integration tests around EQL sequence signal generation

* Clearer assertion

* Simplify test assertion

* Fix typings

These were updated on an upstream refactor.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
rylnd and kibanamachine authored Nov 16, 2020
1 parent dac35cf commit c6e984d
Show file tree
Hide file tree
Showing 18 changed files with 245 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
*/

import { of, merge, timer, throwError } from 'rxjs';
import { takeWhile, switchMap, expand, mergeMap, tap } from 'rxjs/operators';
import { map, takeWhile, switchMap, expand, mergeMap, tap } from 'rxjs/operators';
import { ApiResponse } from '@elastic/elasticsearch';

import {
doSearch,
Expand Down Expand Up @@ -35,6 +36,15 @@ export const doPartialSearch = <SearchResponse = any>(
takeWhile((response) => !isCompleteResponse(response), true)
);

export const normalizeEqlResponse = <SearchResponse extends ApiResponse = ApiResponse>() =>
map<SearchResponse, SearchResponse>((eqlResponse) => ({
...eqlResponse,
body: {
...eqlResponse.body,
...eqlResponse,
},
}));

export const throwOnEsError = () =>
mergeMap((r: IKibanaSearchResponse) =>
isErrorResponse(r) ? merge(of(r), throwError(new AbortError())) : of(r)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const getMockEqlResponse = () => ({
sequences: [],
},
},
meta: {},
statusCode: 200,
});

describe('EQL search strategy', () => {
Expand Down Expand Up @@ -193,5 +195,20 @@ describe('EQL search strategy', () => {
expect(requestOptions).toEqual(expect.objectContaining({ ignore: [400] }));
});
});

describe('response', () => {
it('contains a rawResponse field containing the full search response', async () => {
const eqlSearch = await eqlSearchStrategyProvider(mockLogger);
const response = await eqlSearch
.search({ id: 'my-search-id', options: { ignore: [400] } }, {}, mockDeps)
.toPromise();

expect(response).toEqual(
expect.objectContaining({
rawResponse: expect.objectContaining(getMockEqlResponse()),
})
);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import type { Logger } from 'kibana/server';
import type { ApiResponse } from '@elastic/elasticsearch';

import { search } from '../../../../../src/plugins/data/server';
import { doPartialSearch } from '../../common/search/es_search/es_search_rxjs_utils';
import {
doPartialSearch,
normalizeEqlResponse,
} from '../../common/search/es_search/es_search_rxjs_utils';
import { getAsyncOptions, getDefaultSearchParams } from './get_default_search_params';

import type { ISearchStrategy, IEsRawSearchResponse } from '../../../../../src/plugins/data/server';
Expand Down Expand Up @@ -64,7 +67,7 @@ export const eqlSearchStrategyProvider = (
(response) => response.body.id,
request.id,
options
).pipe(utils.toKibanaSearchResponse());
).pipe(normalizeEqlResponse(), utils.toKibanaSearchResponse());
},
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ export const EQL_TYPE = '[data-test-subj="eqlRuleType"]';

export const EQL_QUERY_INPUT = '[data-test-subj="eqlQueryBarTextInput"]';

export const EQL_QUERY_PREVIEW_HISTOGRAM = '[data-test-subj="queryPreviewEqlHistogram"]';

export const EQL_QUERY_VALIDATION_SPINNER = '[data-test-subj="eql-validation-loading"]';

export const IMPORT_QUERY_FROM_SAVED_TIMELINE_LINK =
'[data-test-subj="importQueryFromSavedTimeline"]';

Expand Down Expand Up @@ -80,6 +84,8 @@ export const MITRE_TACTIC_DROPDOWN = '[data-test-subj="mitreTactic"]';
export const MITRE_TECHNIQUES_INPUT =
'[data-test-subj="mitreTechniques"] [data-test-subj="comboBoxSearchInput"]';

export const QUERY_PREVIEW_BUTTON = '[data-test-subj="queryPreviewButton"]';

export const REFERENCE_URLS_INPUT =
'[data-test-subj="detectionEngineStepAboutRuleReferenceUrls"] input';

Expand Down
9 changes: 9 additions & 0 deletions x-pack/plugins/security_solution/cypress/screens/shared.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

export const NOTIFICATION_TOASTS = '[data-test-subj="globalToastList"]';

export const TOAST_ERROR_CLASS = 'euiToast--danger';
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@ import {
THRESHOLD_TYPE,
EQL_TYPE,
EQL_QUERY_INPUT,
QUERY_PREVIEW_BUTTON,
EQL_QUERY_PREVIEW_HISTOGRAM,
EQL_QUERY_VALIDATION_SPINNER,
} from '../screens/create_new_rule';
import { NOTIFICATION_TOASTS, TOAST_ERROR_CLASS } from '../screens/shared';
import { TIMELINE } from '../screens/timelines';
import { refreshPage } from './security_header';

Expand Down Expand Up @@ -225,8 +229,12 @@ export const fillDefineThresholdRuleAndContinue = (rule: ThresholdRule) => {

export const fillDefineEqlRuleAndContinue = (rule: CustomRule) => {
cy.get(EQL_QUERY_INPUT).type(rule.customQuery);
cy.get(DEFINE_CONTINUE_BUTTON).should('exist').click({ force: true });
cy.get(EQL_QUERY_VALIDATION_SPINNER).should('not.exist');
cy.get(QUERY_PREVIEW_BUTTON).should('not.be.disabled').click({ force: true });
cy.get(EQL_QUERY_PREVIEW_HISTOGRAM).should('contain.text', 'Hits');
cy.get(NOTIFICATION_TOASTS).children().should('not.have.class', TOAST_ERROR_CLASS); // asserts no error toast on page

cy.get(DEFINE_CONTINUE_BUTTON).should('exist').click({ force: true });
cy.get(EQL_QUERY_INPUT).should('not.exist');
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ export const validateEql = async ({
const { rawResponse: response } = await data.search
.search<EqlSearchStrategyRequest, EqlSearchStrategyResponse>(
{
// @ts-expect-error allow_no_indices is missing on EqlSearch
params: { allow_no_indices: true, index: index.join(), body: { query, size: 0 } },
params: { index: index.join(), body: { query, size: 0 } },
options: { ignore: [400] },
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ export const useEqlPreview = (): [
.search<EqlSearchStrategyRequest, EqlSearchStrategyResponse<EqlSearchResponse<Source>>>(
{
params: {
// @ts-expect-error allow_no_indices is missing on EqlSearch
allow_no_indices: true,
index: index.join(),
body: {
filter: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('PreviewCustomQueryHistogram', () => {

expect(wrapper.find('[data-test-subj="queryPreviewLoading"]').exists()).toBeTruthy();
expect(
wrapper.find('[data-test-subj="queryPreviewCustomHistogram"]').at(0).prop('subtitle')
wrapper.find('[dataTestSubj="queryPreviewCustomHistogram"]').at(0).prop('subtitle')
).toEqual(i18n.QUERY_PREVIEW_SUBTITLE_LOADING);
});

Expand All @@ -78,32 +78,32 @@ describe('PreviewCustomQueryHistogram', () => {

expect(wrapper.find('[data-test-subj="queryPreviewLoading"]').exists()).toBeFalsy();
expect(
wrapper.find('[data-test-subj="queryPreviewCustomHistogram"]').at(0).prop('subtitle')
wrapper.find('[dataTestSubj="queryPreviewCustomHistogram"]').at(0).prop('subtitle')
).toEqual(i18n.QUERY_PREVIEW_TITLE(9154));
expect(
wrapper.find('[data-test-subj="queryPreviewCustomHistogram"]').at(0).props().data
).toEqual([
{
key: 'hits',
value: [
{
g: 'All others',
x: 1602247050000,
y: 2314,
},
{
g: 'All others',
x: 1602247162500,
y: 3471,
},
{
g: 'All others',
x: 1602247275000,
y: 3369,
},
],
},
]);
expect(wrapper.find('[dataTestSubj="queryPreviewCustomHistogram"]').at(0).props().data).toEqual(
[
{
key: 'hits',
value: [
{
g: 'All others',
x: 1602247050000,
y: 2314,
},
{
g: 'All others',
x: 1602247162500,
y: 3471,
},
{
g: 'All others',
x: 1602247275000,
y: 3369,
},
],
},
]
);
});

test('it invokes setQuery with id, inspect, isLoading and refetch', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export const PreviewCustomQueryHistogram = ({
subtitle={subtitle}
disclaimer={i18n.QUERY_PREVIEW_DISCLAIMER}
isLoading={isLoading}
data-test-subj="queryPreviewCustomHistogram"
dataTestSubj="queryPreviewCustomHistogram"
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('PreviewEqlQueryHistogram', () => {

expect(wrapper.find('[data-test-subj="queryPreviewLoading"]').exists()).toBeTruthy();
expect(
wrapper.find('[data-test-subj="queryPreviewEqlHistogram"]').at(0).prop('subtitle')
wrapper.find('[dataTestSubj="queryPreviewEqlHistogram"]').at(0).prop('subtitle')
).toEqual(i18n.QUERY_PREVIEW_SUBTITLE_LOADING);
});

Expand All @@ -78,9 +78,9 @@ describe('PreviewEqlQueryHistogram', () => {

expect(wrapper.find('[data-test-subj="queryPreviewLoading"]').exists()).toBeFalsy();
expect(
wrapper.find('[data-test-subj="queryPreviewEqlHistogram"]').at(0).prop('subtitle')
wrapper.find('[dataTestSubj="queryPreviewEqlHistogram"]').at(0).prop('subtitle')
).toEqual(i18n.QUERY_PREVIEW_TITLE(9154));
expect(wrapper.find('[data-test-subj="queryPreviewEqlHistogram"]').at(0).props().data).toEqual([
expect(wrapper.find('[dataTestSubj="queryPreviewEqlHistogram"]').at(0).props().data).toEqual([
{
key: 'hits',
value: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export const PreviewEqlQueryHistogram = ({
subtitle={subtitle}
disclaimer={i18n.QUERY_PREVIEW_DISCLAIMER_EQL}
isLoading={isLoading}
data-test-subj="queryPreviewEqlHistogram"
dataTestSubj="queryPreviewEqlHistogram"
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const LoadingChart = styled(EuiLoadingChart)`
interface PreviewHistogramProps {
id: string;
data: ChartSeriesData[];
dataTestSubj?: string;
barConfig: ChartSeriesConfigs;
title: string;
subtitle: string;
Expand All @@ -31,6 +32,7 @@ interface PreviewHistogramProps {
export const PreviewHistogram = ({
id,
data,
dataTestSubj,
barConfig,
title,
subtitle,
Expand All @@ -39,7 +41,7 @@ export const PreviewHistogram = ({
}: PreviewHistogramProps) => {
return (
<>
<Panel height={300}>
<Panel height={300} data-test-subj={dataTestSubj}>
<EuiFlexGroup gutterSize="none" direction="column">
<EuiFlexItem grow={1}>
<HeaderSection id={id} title={title} titleSize="xs" subtitle={subtitle} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ describe('PreviewQuery', () => {
expect(
wrapper.find('[data-test-subj="queryPreviewButton"] button').props().disabled
).toBeFalsy();
expect(wrapper.find('[data-test-subj="previewNonEqlQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="previewThresholdQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="previewEqlQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewCustomHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="thresholdQueryPreviewHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewEqlHistogram"]').exists()).toBeFalsy();
});

test('it renders preview button disabled if "isDisabled" is true', () => {
Expand Down Expand Up @@ -146,9 +146,9 @@ describe('PreviewQuery', () => {
const mockCalls = (useKibana().services.data.search.search as jest.Mock).mock.calls;

expect(mockCalls.length).toEqual(1);
expect(wrapper.find('[data-test-subj="previewNonEqlQueryHistogram"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="previewThresholdQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="previewEqlQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewCustomHistogram"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="thresholdQueryPreviewHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewEqlHistogram"]').exists()).toBeFalsy();
});

test('it renders noise warning when rule type is query, timeframe is last hour and hit average is greater than 1/hour', async () => {
Expand Down Expand Up @@ -209,9 +209,9 @@ describe('PreviewQuery', () => {
const mockCalls = (useKibana().services.data.search.search as jest.Mock).mock.calls;

expect(mockCalls.length).toEqual(1);
expect(wrapper.find('[data-test-subj="previewNonEqlQueryHistogram"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="previewThresholdQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="previewEqlQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewCustomHistogram"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="thresholdQueryPreviewHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewEqlHistogram"]').exists()).toBeFalsy();
});

test('it renders eql histogram when preview button clicked and rule type is eql', () => {
Expand All @@ -236,9 +236,9 @@ describe('PreviewQuery', () => {
const mockCalls = (useKibana().services.data.search.search as jest.Mock).mock.calls;

expect(mockCalls.length).toEqual(1);
expect(wrapper.find('[data-test-subj="previewNonEqlQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="previewThresholdQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="previewEqlQueryHistogram"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="queryPreviewCustomHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="thresholdQueryPreviewHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewEqlHistogram"]').exists()).toBeTruthy();
});

test('it renders noise warning when rule type is eql, timeframe is last hour and hit average is greater than 1/hour', async () => {
Expand Down Expand Up @@ -314,9 +314,9 @@ describe('PreviewQuery', () => {

expect(mockCalls.length).toEqual(1);
expect(wrapper.find('[data-test-subj="previewQueryWarning"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="previewNonEqlQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="previewThresholdQueryHistogram"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="previewEqlQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewCustomHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="thresholdQueryPreviewHistogram"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="queryPreviewEqlHistogram"]').exists()).toBeFalsy();
});

test('it renders noise warning when rule type is threshold, and threshold field is defined, timeframe is last hour and hit average is greater than 1/hour', async () => {
Expand Down Expand Up @@ -380,9 +380,9 @@ describe('PreviewQuery', () => {
const mockCalls = (useKibana().services.data.search.search as jest.Mock).mock.calls;

expect(mockCalls.length).toEqual(1);
expect(wrapper.find('[data-test-subj="previewNonEqlQueryHistogram"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="previewThresholdQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="previewEqlQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewCustomHistogram"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="thresholdQueryPreviewHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewEqlHistogram"]').exists()).toBeFalsy();
});

test('it renders query histogram when preview button clicked, rule type is threshold, and threshold field is empty string', () => {
Expand All @@ -407,9 +407,9 @@ describe('PreviewQuery', () => {
const mockCalls = (useKibana().services.data.search.search as jest.Mock).mock.calls;

expect(mockCalls.length).toEqual(1);
expect(wrapper.find('[data-test-subj="previewNonEqlQueryHistogram"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="previewThresholdQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="previewEqlQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewCustomHistogram"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="thresholdQueryPreviewHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewEqlHistogram"]').exists()).toBeFalsy();
});

test('it hides histogram when timeframe changes', () => {
Expand All @@ -431,13 +431,13 @@ describe('PreviewQuery', () => {

wrapper.find('[data-test-subj="queryPreviewButton"] button').at(0).simulate('click');

expect(wrapper.find('[data-test-subj="previewNonEqlQueryHistogram"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="queryPreviewCustomHistogram"]').exists()).toBeTruthy();

wrapper
.find('[data-test-subj="queryPreviewTimeframeSelect"] select')
.at(0)
.simulate('change', { target: { value: 'd' } });

expect(wrapper.find('[data-test-subj="previewNonEqlQueryHistogram"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="queryPreviewCustomHistogram"]').exists()).toBeFalsy();
});
});
Loading

0 comments on commit c6e984d

Please sign in to comment.