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.

Sign up for GitHub

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, Lists] Replace legacy imports from 'elasticsearch' package #107226

Merged
merged 10 commits into from
Aug 5, 2021
Merged

[Security Solution, Lists] Replace legacy imports from 'elasticsearch' package #107226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
74d09fe
Remove legacy imports from 'elasticsearch' package
rylnd Jul 29, 2021
a108af5
Fix type errors with a null guard
rylnd Jul 29, 2021
8746a88
Fix a few more type errors
rylnd Jul 29, 2021
ef54ff6
Replace legacy type imports in integration tests
rylnd Aug 2, 2021
6c9bc3e
Merge branch 'master' into replace-legacy-secsol-imports
rylnd Aug 2, 2021
93aa2db
Update more integration tests to account for our optional _source
rylnd Aug 2, 2021
e2f31cb
Fix more type errors
rylnd Aug 2, 2021
765e6c9
Merge branch 'master' into replace-legacy-secsol-imports
kibanamachine Aug 3, 2021
34fb078
Merge branch 'master' into replace-legacy-secsol-imports
rylnd Aug 4, 2021
63aa115
Merge branch 'master' into replace-legacy-secsol-imports
kibanamachine Aug 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions x-pack/plugins/lists/server/schemas/common/get_shard.mock.ts
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions x-pack/plugins/lists/server/schemas/elastic_response/search_es_list_item_schema.mock.ts
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions x-pack/plugins/lists/server/schemas/elastic_response/search_es_list_schema.mock.ts
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions x-pack/plugins/lists/server/services/items/find_list_item.mock.ts
View file
Open in desktop
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
4 changes: 3 additions & 1 deletion x-pack/plugins/lists/server/services/items/write_list_items_to_stream.test.ts
View file
Open in desktop
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
12 changes: 8 additions & 4 deletions x-pack/plugins/lists/server/services/utils/transform_elastic_to_list_item.test.ts
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions .../plugins/security_solution/public/common/components/exceptions/use_add_exception.test.tsx
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.tsx
View file
Open in desktop
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';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nifty

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
8 changes: 6 additions & 2 deletions x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx
View file
Open in desktop
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't this not run if response.version_conflicts === 0 since thats a falsy value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, a value of 0 never satisfied this condition.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure if you wanted it to now given the defaults below (should have added that point). 👍🏾

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see! I think 0 makes sense as the default here, I can't think of a more appropriate one.

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
4 changes: 2 additions & 2 deletions x-pack/plugins/security_solution/public/detections/containers/detection_engine/alerts/api.ts
View file
Open in desktop
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
16 changes: 14 additions & 2 deletions x-pack/plugins/security_solution/public/hosts/components/authentications_table/mock.ts
View file
Open in desktop
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
8 changes: 5 additions & 3 deletions x-pack/plugins/security_solution/server/endpoint/routes/metadata/support/test_support.ts
View file
Open in desktop
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>;
}
6 changes: 3 additions & 3 deletions x-pack/plugins/security_solution/server/endpoint/routes/policy/handlers.test.ts
View file
Open in desktop
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>;
}
7 changes: 3 additions & 4 deletions x-pack/plugins/security_solution/server/endpoint/services/metadata/metadata.ts
View file
Open in desktop
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;
}
14 changes: 7 additions & 7 deletions x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/patch_cases.ts
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions x-pack/test/case_api_integration/security_and_spaces/tests/common/comments/post_comment.ts
View file
Open in desktop
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