Skip to content

Commit

Permalink
[Security Solution, Lists] Replace legacy imports from 'elasticsearch…
Browse files Browse the repository at this point in the history
…' package (elastic#107226)

* Remove legacy imports from 'elasticsearch' package

This prefers the newer types from '@elastic/elasticsearch'.

There was one instance where mock data was insufficient to satisfy the
newer analogous types; in all other cases this was just a find/replace.

* Fix type errors with a null guard

We know that this mock has hits with _source values, but we cannot
convey this to typescript as null assertions are disabled within this
project. This seems like the next best solution, preferable to a
@ts-expect-error.

* Fix a few more type errors

* Replace legacy type imports in integration tests

* refactors destructuring due to _source being properly declared as
  conditional

* Update more integration tests to account for our optional _source

Changes here fall into one of two categories:

* If the test was making an assertion on a value from _source, we simply
null chain and continue to assert on a possibly undefined value.

* If the test logic depends on _source being present, we first assert that
presence, and exit the test early if absent.

* Fix more type errors

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
rylnd and kibanamachine committed Aug 5, 2021
1 parent 3a67301 commit c6b3f51
Show file tree
Hide file tree
Showing 41 changed files with 532 additions and 485 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugins/lists/server/schemas/common/get_shard.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
* 2.0.
*/

import { ShardsResponse } from 'elasticsearch';
import type { estypes } from '@elastic/elasticsearch';

export const getShardMock = (): ShardsResponse => ({
export const getShardMock = (): estypes.ShardStatistics => ({
failed: 0,
skipped: 0,
successful: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { SearchResponse } from 'elasticsearch';
import type { estypes } from '@elastic/elasticsearch';

import {
DATE_NOW,
Expand Down Expand Up @@ -61,7 +61,7 @@ export const getSearchEsListItemMock = (): SearchEsListItemSchema => ({
ip: VALUE,
});

export const getSearchListItemMock = (): SearchResponse<SearchEsListItemSchema> => ({
export const getSearchListItemMock = (): estypes.SearchResponse<SearchEsListItemSchema> => ({
_scroll_id: '123',
_shards: getShardMock(),
hits: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { SearchResponse } from 'elasticsearch';
import type { estypes } from '@elastic/elasticsearch';

import {
DATE_NOW,
Expand Down Expand Up @@ -40,7 +40,7 @@ export const getSearchEsListMock = (): SearchEsListSchema => ({
version: VERSION,
});

export const getSearchListMock = (): SearchResponse<SearchEsListSchema> => ({
export const getSearchListMock = (): estypes.SearchResponse<SearchEsListSchema> => ({
_scroll_id: '123',
_shards: getShardMock(),
hits: {
Expand All @@ -60,7 +60,7 @@ export const getSearchListMock = (): SearchResponse<SearchEsListSchema> => ({
took: 10,
});

export const getEmptySearchListMock = (): SearchResponse<SearchEsListSchema> => ({
export const getEmptySearchListMock = (): estypes.SearchResponse<SearchEsListSchema> => ({
_scroll_id: '123',
_shards: getShardMock(),
hits: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { Client } from 'elasticsearch';
import type { estypes } from '@elastic/elasticsearch';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks';

Expand All @@ -14,7 +14,7 @@ import { getShardMock } from '../../schemas/common/get_shard.mock';

import { FindListItemOptions } from './find_list_item';

export const getFindCount = (): ReturnType<Client['count']> => {
export const getFindCount = (): Promise<estypes.CountResponse> => {
return Promise.resolve({
_shards: getShardMock(),
count: 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ describe('write_list_items_to_stream', () => {
firstResponse.hits.hits[0].sort = ['some-sort-value'];

const secondResponse = getSearchListItemMock();
secondResponse.hits.hits[0]._source.ip = '255.255.255.255';
if (secondResponse.hits.hits[0]._source) {
secondResponse.hits.hits[0]._source.ip = '255.255.255.255';
}

const esClient = elasticsearchClientMock.createScopedClusterClient().asCurrentUser;
esClient.search.mockResolvedValueOnce(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ describe('transform_elastic_to_list_item', () => {

test('it transforms an elastic keyword type to a list item type', () => {
const response = getSearchListItemMock();
response.hits.hits[0]._source.ip = undefined;
response.hits.hits[0]._source.keyword = 'host-name-example';
if (response.hits.hits[0]._source) {
response.hits.hits[0]._source.ip = undefined;
response.hits.hits[0]._source.keyword = 'host-name-example';
}
const queryFilter = transformElasticToListItem({
response,
type: 'keyword',
Expand Down Expand Up @@ -68,8 +70,10 @@ describe('transform_elastic_to_list_item', () => {
const {
hits: { hits },
} = getSearchListItemMock();
hits[0]._source.ip = undefined;
hits[0]._source.keyword = 'host-name-example';
if (hits[0]._source) {
hits[0]._source.ip = undefined;
hits[0]._source.keyword = 'host-name-example';
}
const queryFilter = transformElasticHitsToListItem({
hits,
type: 'keyword',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { act, renderHook, RenderHookResult } from '@testing-library/react-hooks';
import type { estypes } from '@elastic/elasticsearch';
import { coreMock } from '../../../../../../../src/core/public/mocks';
import { KibanaServices } from '../../../common/lib/kibana';

Expand All @@ -28,7 +29,6 @@ import {
ReturnUseAddOrUpdateException,
AddOrUpdateExceptionItemsFunc,
} from './use_add_exception';
import { UpdateDocumentByQueryResponse } from 'elasticsearch';

const mockKibanaHttpService = coreMock.createStart().http;
const mockKibanaServices = KibanaServices.get as jest.Mock;
Expand All @@ -39,7 +39,7 @@ const fetchMock = jest.fn();
mockKibanaServices.mockReturnValue({ http: { fetch: fetchMock } });

describe('useAddOrUpdateException', () => {
let updateAlertStatus: jest.SpyInstance<Promise<UpdateDocumentByQueryResponse>>;
let updateAlertStatus: jest.SpyInstance<Promise<estypes.UpdateByQueryResponse>>;
let addExceptionListItem: jest.SpyInstance<Promise<ExceptionListItemSchema>>;
let updateExceptionListItem: jest.SpyInstance<Promise<ExceptionListItemSchema>>;
let getQueryFilter: jest.SpyInstance<ReturnType<typeof getQueryFilterHelper.getQueryFilter>>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { useEffect, useRef, useState, useCallback } from 'react';
import { UpdateDocumentByQueryResponse } from 'elasticsearch';
import type { estypes } from '@elastic/elasticsearch';
import type {
ExceptionListItemSchema,
CreateExceptionListItemSchema,
Expand Down Expand Up @@ -120,8 +120,8 @@ export const useAddOrUpdateException = ({

try {
setIsLoading(true);
let alertIdResponse: UpdateDocumentByQueryResponse | undefined;
let bulkResponse: UpdateDocumentByQueryResponse | undefined;
let alertIdResponse: estypes.UpdateByQueryResponse | undefined;
let bulkResponse: estypes.UpdateByQueryResponse | undefined;
if (alertIdToClose != null) {
alertIdResponse = await updateAlertStatus({
query: getUpdateAlertsQuery([alertIdToClose]),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export const updateAlertStatusAction = async ({
// TODO: Only delete those that were successfully updated from updatedRules
setEventsDeleted({ eventIds: alertIds, isDeleted: true });

if (response.version_conflicts > 0 && alertIds.length === 1) {
if (response.version_conflicts && alertIds.length === 1) {
throw new Error(
i18n.translate(
'xpack.securitySolution.detectionEngine.alerts.updateAlertStatusFailedSingleAlert',
Expand All @@ -105,7 +105,11 @@ export const updateAlertStatusAction = async ({
);
}

onAlertStatusUpdateSuccess(response.updated, response.version_conflicts, selectedStatus);
onAlertStatusUpdateSuccess(
response.updated ?? 0,
response.version_conflicts ?? 0,
selectedStatus
);
} catch (error) {
onAlertStatusUpdateFailure(selectedStatus, error);
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { UpdateDocumentByQueryResponse } from 'elasticsearch';
import type { estypes } from '@elastic/elasticsearch';
import { getCasesFromAlertsUrl } from '../../../../../../cases/common';
import { HostIsolationResponse, HostInfo } from '../../../../../common/endpoint/types';
import {
Expand Down Expand Up @@ -62,7 +62,7 @@ export const updateAlertStatus = async ({
query,
status,
signal,
}: UpdateAlertStatusProps): Promise<UpdateDocumentByQueryResponse> =>
}: UpdateAlertStatusProps): Promise<estypes.UpdateByQueryResponse> =>
KibanaServices.get().http.fetch(DETECTION_ENGINE_SIGNALS_STATUS_URL, {
method: 'POST',
body: JSON.stringify({ conflicts: 'proceed', status, ...query }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,24 @@
* 2.0.
*/

import { SearchResponse } from 'elasticsearch';
import type { estypes } from '@elastic/elasticsearch';
import { HostAuthenticationsStrategyResponse } from '../../../../common/search_strategy/security_solution/hosts/authentications';

export const mockData: { Authentications: HostAuthenticationsStrategyResponse } = {
Authentications: {
rawResponse: {
took: 880,
timed_out: false,
_shards: {
total: 26,
successful: 26,
skipped: 0,
failed: 0,
},
hits: {
total: 2,
hits: [],
},
aggregations: {
group_by_users: {
buckets: [
Expand All @@ -32,7 +44,7 @@ export const mockData: { Authentications: HostAuthenticationsStrategyResponse }
sum_other_doc_count: 566,
},
},
} as SearchResponse<unknown>,
} as estypes.SearchResponse<unknown>,
totalCount: 54,
edges: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
* 2.0.
*/

import { SearchResponse } from 'elasticsearch';
import type { estypes } from '@elastic/elasticsearch';
import { HostMetadata } from '../../../../../common/endpoint/types';

export function createV2SearchResponse(hostMetadata?: HostMetadata): SearchResponse<HostMetadata> {
export function createV2SearchResponse(
hostMetadata?: HostMetadata
): estypes.SearchResponse<HostMetadata> {
return ({
took: 15,
timed_out: false,
Expand Down Expand Up @@ -38,5 +40,5 @@ export function createV2SearchResponse(hostMetadata?: HostMetadata): SearchRespo
]
: [],
},
} as unknown) as SearchResponse<HostMetadata>;
} as unknown) as estypes.SearchResponse<HostMetadata>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
loggingSystemMock,
savedObjectsClientMock,
} from '../../../../../../../src/core/server/mocks';
import { SearchResponse } from 'elasticsearch';
import type { estypes } from '@elastic/elasticsearch';
import { GetHostPolicyResponse, HostPolicyResponse } from '../../../../common/endpoint/types';
import { EndpointDocGenerator } from '../../../../common/endpoint/generate_data';
import { parseExperimentalConfigValue } from '../../../../common/experimental_features';
Expand Down Expand Up @@ -239,7 +239,7 @@ describe('test policy response handler', () => {
*/
function createSearchResponse(
hostPolicyResponse?: HostPolicyResponse
): SearchResponse<HostPolicyResponse> {
): estypes.SearchResponse<HostPolicyResponse> {
return ({
took: 15,
timed_out: false,
Expand Down Expand Up @@ -267,5 +267,5 @@ function createSearchResponse(
]
: [],
},
} as unknown) as SearchResponse<HostPolicyResponse>;
} as unknown) as estypes.SearchResponse<HostPolicyResponse>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
* 2.0.
*/

import { SearchRequest } from '@elastic/elasticsearch/api/types';
import { SearchResponse } from 'elasticsearch';
import type { estypes } from '@elastic/elasticsearch';
import { HostMetadata } from '../../../../common/endpoint/types';
import { SecuritySolutionRequestHandlerContext } from '../../../types';
import { getESQueryHostMetadataByIDs } from '../../routes/metadata/query_builders';
Expand All @@ -20,7 +19,7 @@ export async function getMetadataForEndpoints(
): Promise<HostMetadata[]> {
const query = getESQueryHostMetadataByIDs(endpointIDs);
const esClient = requestHandlerContext.core.elasticsearch.client.asCurrentUser;
const { body } = await esClient.search<HostMetadata>(query as SearchRequest);
const hosts = queryResponseToHostListResult(body as SearchResponse<HostMetadata>);
const { body } = await esClient.search<HostMetadata>(query as estypes.SearchRequest);
const hosts = queryResponseToHostListResult(body as estypes.SearchResponse<HostMetadata>);
return hosts.resultList;
}
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ export default ({ getService }: FtrProviderContext): void => {
const signals = await getSignalsByIds(supertest, [id]);

const alert = signals.hits.hits[0];
expect(alert._source.signal.status).eql('open');
expect(alert._source?.signal.status).eql('open');

const caseUpdated = await createComment({
supertest,
Expand Down Expand Up @@ -846,7 +846,7 @@ export default ({ getService }: FtrProviderContext): void => {
.send(getQuerySignalIds([alert._id]))
.expect(200);

expect(updatedAlert.hits.hits[0]._source.signal.status).eql('in-progress');
expect(updatedAlert.hits.hits[0]._source?.signal.status).eql('in-progress');
});

it('does NOT updates alert status when the status is updated and syncAlerts=false', async () => {
Expand All @@ -863,7 +863,7 @@ export default ({ getService }: FtrProviderContext): void => {
const signals = await getSignalsByIds(supertest, [id]);

const alert = signals.hits.hits[0];
expect(alert._source.signal.status).eql('open');
expect(alert._source?.signal.status).eql('open');

const caseUpdated = await createComment({
supertest,
Expand Down Expand Up @@ -899,7 +899,7 @@ export default ({ getService }: FtrProviderContext): void => {
.send(getQuerySignalIds([alert._id]))
.expect(200);

expect(updatedAlert.hits.hits[0]._source.signal.status).eql('open');
expect(updatedAlert.hits.hits[0]._source?.signal.status).eql('open');
});

it('it updates alert status when syncAlerts is turned on', async () => {
Expand All @@ -916,7 +916,7 @@ export default ({ getService }: FtrProviderContext): void => {
const signals = await getSignalsByIds(supertest, [id]);

const alert = signals.hits.hits[0];
expect(alert._source.signal.status).eql('open');
expect(alert._source?.signal.status).eql('open');

const caseUpdated = await createComment({
supertest,
Expand Down Expand Up @@ -970,7 +970,7 @@ export default ({ getService }: FtrProviderContext): void => {
.send(getQuerySignalIds([alert._id]))
.expect(200);

expect(updatedAlert.hits.hits[0]._source.signal.status).eql('in-progress');
expect(updatedAlert.hits.hits[0]._source?.signal.status).eql('in-progress');
});

it('it does NOT updates alert status when syncAlerts is turned off', async () => {
Expand All @@ -983,7 +983,7 @@ export default ({ getService }: FtrProviderContext): void => {
const signals = await getSignalsByIds(supertest, [id]);

const alert = signals.hits.hits[0];
expect(alert._source.signal.status).eql('open');
expect(alert._source?.signal.status).eql('open');

const caseUpdated = await createComment({
supertest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ export default ({ getService }: FtrProviderContext): void => {
const signals = await getSignalsByIds(supertest, [id]);

const alert = signals.hits.hits[0];
expect(alert._source.signal.status).eql('open');
expect(alert._source?.signal.status).eql('open');

await createComment({
supertest,
Expand Down Expand Up @@ -424,7 +424,7 @@ export default ({ getService }: FtrProviderContext): void => {
const signals = await getSignalsByIds(supertest, [id]);

const alert = signals.hits.hits[0];
expect(alert._source.signal.status).eql('open');
expect(alert._source?.signal.status).eql('open');

await createComment({
supertest,
Expand Down
Loading

0 comments on commit c6b3f51

Please sign in to comment.