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 Solutions] Fixes jest test memory leaks within the server side detection engine #117964

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/server/config.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
ExperimentalFeatures,
parseExperimentalConfigValue,
} from '../common/experimental_features';
import { ConfigType } from './config';
import type { ConfigType } from './config';
import { UnderlyingLogClient } from './lib/detection_engine/rule_execution_log/types';

export const createMockConfig = (): ConfigType => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
AgentPolicyServiceInterface,
PackagePolicyServiceInterface,
} from '../../../fleet/server';
import { PluginStartContract as AlertsPluginStartContract } from '../../../alerting/server';
import type { PluginStartContract as AlertsPluginStartContract } from '../../../alerting/server';
import {
getPackagePolicyCreateCallback,
getPackagePolicyUpdateCallback,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { KibanaRequest, Logger, RequestHandlerContext } from 'kibana/server';
import { ExceptionListClient } from '../../../lists/server';
import { PluginStartContract as AlertsStartContract } from '../../../alerting/server';
import type { PluginStartContract as AlertsStartContract } from '../../../alerting/server';
import {
PostPackagePolicyCreateCallback,
PostPackagePolicyDeleteCallback,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { KibanaRequest, Logger } from 'kibana/server';
import { ExceptionListClient } from '../../../../lists/server';
import { PluginStartContract as AlertsStartContract } from '../../../../alerting/server';
import type { PluginStartContract as AlertsStartContract } from '../../../../alerting/server';
import { createDetectionIndex } from '../../lib/detection_engine/routes/index/create_index_route';
import { createPrepackagedRules } from '../../lib/detection_engine/routes/rules/add_prepackaged_rules_route';
import { SecuritySolutionApiRequestHandlerContext } from '../../types';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { SanitizedAlert } from '../../../../../alerting/common';
import type { SanitizedAlert } from '../../../../../alerting/common';
import { SERVER_APP_ID, LEGACY_NOTIFICATIONS_ID } from '../../../../common/constants';
// eslint-disable-next-line no-restricted-imports
import { CreateNotificationParams, LegacyRuleNotificationAlertTypeParams } from './legacy_types';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { AlertTypeParams, FindResult } from '../../../../../alerting/server';
import type { AlertTypeParams, FindResult } from '../../../../../alerting/server';
import { LEGACY_NOTIFICATIONS_ID } from '../../../../common/constants';
// eslint-disable-next-line no-restricted-imports
import { LegacyFindNotificationParams } from './legacy_types';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { AlertTypeParams, SanitizedAlert } from '../../../../../alerting/common';
import type { AlertTypeParams, SanitizedAlert } from '../../../../../alerting/common';
// eslint-disable-next-line no-restricted-imports
import { LegacyReadNotificationParams, legacyIsAlertType } from './legacy_types';
// eslint-disable-next-line no-restricted-imports
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { Logger } from 'src/core/server';
import type { Logger } from 'src/core/server';
import { parseScheduleDates } from '@kbn/securitysolution-io-ts-utils';
import {
DEFAULT_RULE_NOTIFICATION_QUERY_SIZE,
Expand All @@ -18,7 +18,7 @@ import {
LegacyNotificationAlertTypeDefinition,
legacyRulesNotificationParams,
} from './legacy_types';
import { AlertAttributes } from '../signals/types';
import type { AlertAttributes } from '../signals/types';
import { siemRuleActionGroups } from '../signals/siem_rule_action_groups';
import { scheduleNotificationActions } from './schedule_notification_actions';
import { getNotificationResultsLink } from './utils';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { loggingSystemMock } from 'src/core/server/mocks';
// eslint-disable-next-line no-restricted-imports
import { LegacyRulesNotificationParams } from '../legacy_types';
import type { LegacyRulesNotificationParams } from '../legacy_types';
// eslint-disable-next-line no-restricted-imports
import { legacyExtractReferences } from './legacy_extract_references';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
* 2.0.
*/

import { Logger } from 'src/core/server';
import { RuleParamsAndRefs } from '../../../../../../alerting/server';
import type { Logger } from 'src/core/server';
import type { RuleParamsAndRefs } from '../../../../../../alerting/server';
// eslint-disable-next-line no-restricted-imports
import { LegacyRulesNotificationParams } from '../legacy_types';
import type { LegacyRulesNotificationParams } from '../legacy_types';
// eslint-disable-next-line no-restricted-imports
import { legacyExtractRuleId } from './legacy_extract_rule_id';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { loggingSystemMock } from 'src/core/server/mocks';
// eslint-disable-next-line no-restricted-imports
import { LegacyRulesNotificationParams } from '../legacy_types';
import type { LegacyRulesNotificationParams } from '../legacy_types';
// eslint-disable-next-line no-restricted-imports
import { legacyExtractRuleId } from './legacy_extract_rule_id';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { Logger, SavedObjectReference } from 'src/core/server';
// eslint-disable-next-line no-restricted-imports
import { legacyGetRuleReference } from '../../rule_actions/legacy_utils';
// eslint-disable-next-line no-restricted-imports
import { LegacyRulesNotificationParams } from '../legacy_types';
import type { LegacyRulesNotificationParams } from '../legacy_types';

/**
* This extracts the "ruleAlertId" "id" and returns it as a saved object reference.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
*/

import { loggingSystemMock } from 'src/core/server/mocks';
import { SavedObjectReference } from 'src/core/server';
import type { SavedObjectReference } from 'src/core/server';

// eslint-disable-next-line no-restricted-imports
import { LegacyRulesNotificationParams } from '../legacy_types';
import type { LegacyRulesNotificationParams } from '../legacy_types';
// eslint-disable-next-line no-restricted-imports
import { legacyInjectReferences } from './legacy_inject_references';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { Logger, SavedObjectReference } from 'src/core/server';
// eslint-disable-next-line no-restricted-imports
import { LegacyRulesNotificationParams } from '../legacy_types';
import type { LegacyRulesNotificationParams } from '../legacy_types';
// eslint-disable-next-line no-restricted-imports
import { legacyInjectRuleIdReferences } from './legacy_inject_rule_id_references';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
*/

import { loggingSystemMock } from 'src/core/server/mocks';
import { SavedObjectReference } from 'src/core/server';
import type { SavedObjectReference } from 'src/core/server';

// eslint-disable-next-line no-restricted-imports
import { legacyInjectRuleIdReferences } from './legacy_inject_rule_id_references';
// eslint-disable-next-line no-restricted-imports
import { LegacyRulesNotificationParams } from '../legacy_types';
import type { LegacyRulesNotificationParams } from '../legacy_types';

describe('legacy_inject_rule_id_references', () => {
type FuncReturn = ReturnType<typeof legacyInjectRuleIdReferences>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { Logger, SavedObjectReference } from 'src/core/server';
// eslint-disable-next-line no-restricted-imports
import { LegacyRulesNotificationParams } from '../legacy_types';
import type { LegacyRulesNotificationParams } from '../legacy_types';

/**
* This injects any legacy "id"'s from saved object reference and returns the "ruleAlertId" using the saved object reference. If for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { schema, TypeOf } from '@kbn/config-schema';

import {
import type {
RulesClient,
PartialAlert,
AlertType,
Expand All @@ -17,7 +17,7 @@ import {
AlertInstanceContext,
AlertExecutorOptions,
} from '../../../../../alerting/server';
import { Alert, AlertAction } from '../../../../../alerting/common';
import type { Alert, AlertAction } from '../../../../../alerting/common';
import { LEGACY_NOTIFICATIONS_ID } from '../../../../common/constants';

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
*/

import { mapKeys, snakeCase } from 'lodash/fp';
import { AlertInstance } from '../../../../../alerting/server';
import type { AlertInstance } from '../../../../../alerting/server';
import { expandDottedObject } from '../rule_types/utils';
import { RuleParams } from '../schemas/rule_schemas';
import type { RuleParams } from '../schemas/rule_schemas';
import aadFieldConversion from '../routes/index/signal_aad_mapping.json';
import { isRACAlert } from '../signals/utils';
import { RACAlert } from '../rule_types/types';
import type { RACAlert } from '../rule_types/types';

export type NotificationRuleTypeParams = RuleParams & {
id: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@

import { ElasticsearchClient, SavedObject, Logger } from 'src/core/server';
import { parseScheduleDates } from '@kbn/securitysolution-io-ts-utils';
import { AlertInstance } from '../../../../../alerting/server';
import { RuleParams } from '../schemas/rule_schemas';
import type { AlertInstance } from '../../../../../alerting/server';
import type { RuleParams } from '../schemas/rule_schemas';
import { deconflictSignalsAndResults, getNotificationResultsLink } from '../notifications/utils';
import { DEFAULT_RULE_NOTIFICATION_QUERY_SIZE } from '../../../../common/constants';
import { getSignals } from '../notifications/get_signals';
import {
NotificationRuleTypeParams,
scheduleNotificationActions,
} from './schedule_notification_actions';
import { AlertAttributes } from '../signals/types';
import type { AlertAttributes } from '../signals/types';

interface ScheduleThrottledNotificationActionsOptions {
id: SavedObject['id'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
* 2.0.
*/

import { Logger } from 'src/core/server';
import type { Logger } from 'src/core/server';
import { APP_PATH } from '../../../../common/constants';
import { SignalSearchResponse } from '../signals/types';
import type { SignalSearchResponse } from '../signals/types';

export const getNotificationResultsLink = ({
kibanaSiemAppUrl = APP_PATH,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@
import type { MockedKeys } from '@kbn/utility-types/jest';
import { coreMock } from 'src/core/server/mocks';

import { ActionsApiRequestHandlerContext } from '../../../../../../actions/server';
import { AlertingApiRequestHandlerContext } from '../../../../../../alerting/server';
import type { ActionsApiRequestHandlerContext } from '../../../../../../actions/server';
import type { AlertingApiRequestHandlerContext } from '../../../../../../alerting/server';
import { rulesClientMock } from '../../../../../../alerting/server/mocks';
import { actionsClientMock } from '../../../../../../actions/server/mocks';

// We have to disable this path otherwise we leak memory and cannot directly import from "../../../../../../actions/server/"
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { actionsClientMock } from '../../../../../../actions/server/actions_client.mock';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE: I have to use this and cannot use import { actionsClientMock } from '../../../../../../actions/server/mocks'; without having a memory leak within Jest.

Copy link
Member

Choose a reason for hiding this comment

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

Is there anyway we can add a linter rule to this and the above infringing mocks to prevent folks from importing them this way in the future (or warn at least?), or best just to prioritize identifying and fixing the underlying issues? Just curious what we can do to prevent these memory leaks from slowly sneaking their way back in... 🐌💧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am thinking that maybe this isn't the right way for us to go here. I am thinking of putting this back into draft mode and talking to more operations people. The module loader in some cases looks to be re-loading or leaking the modules and handling this at an infrastructure level might be better for us.

import { licensingMock } from '../../../../../../licensing/server/mocks';
import { listMock } from '../../../../../../lists/server/mocks';
import { ruleRegistryMocks } from '../../../../../../rule_registry/server/mocks';
Expand All @@ -27,7 +30,7 @@ import type {
SecuritySolutionRequestHandlerContext,
} from '../../../../types';

const createMockClients = () => {
export const createMockClients = () => {
const core = coreMock.createRequestHandlerContext();
const license = licensingMock.createLicenseMock();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { ALERT_WORKFLOW_STATUS } from '@kbn/rule-data-utils';
import { ruleTypeMappings } from '@kbn/securitysolution-rules';

import { SavedObjectsFindResponse } from 'src/core/server';
import type { SavedObjectsFindResponse } from 'src/core/server';

import { ActionResult } from '../../../../../../actions/server';
import type { ActionResult } from '../../../../../../actions/server';
import {
DETECTION_ENGINE_RULES_URL,
DETECTION_ENGINE_SIGNALS_STATUS_URL,
Expand All @@ -25,21 +25,21 @@ import {
DETECTION_ENGINE_RULES_BULK_ACTION,
INTERNAL_DETECTION_ENGINE_RULE_STATUS_URL,
} from '../../../../../common/constants';
import {
import type {
RuleAlertType,
IRuleSavedAttributesSavedObjectAttributes,
HapiReadableStream,
IRuleStatusSOAttributes,
} from '../../rules/types';
import { requestMock } from './request';
import { QuerySignalsSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/query_signals_index_schema';
import { SetSignalsStatusSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/set_signal_status_schema';
import type { QuerySignalsSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/query_signals_index_schema';
import type { SetSignalsStatusSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/set_signal_status_schema';
import { getCreateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/rule_schemas.mock';
import { getFinalizeSignalsMigrationSchemaMock } from '../../../../../common/detection_engine/schemas/request/finalize_signals_migration_schema.mock';
import { EqlSearchResponse } from '../../../../../common/detection_engine/types';
import type { EqlSearchResponse } from '../../../../../common/detection_engine/types';
import { getSignalsMigrationStatusSchemaMock } from '../../../../../common/detection_engine/schemas/request/get_signals_migration_status_schema.mock';
import { RuleParams } from '../../schemas/rule_schemas';
import { SanitizedAlert, ResolvedSanitizedRule } from '../../../../../../alerting/common';
import type { RuleParams } from '../../schemas/rule_schemas';
import type { SanitizedAlert, ResolvedSanitizedRule } from '../../../../../../alerting/common';
import { getQueryRuleParams } from '../../schemas/rule_schemas.mock';
import { getPerformBulkActionSchemaMock } from '../../../../../common/detection_engine/schemas/request/perform_bulk_action_schema.mock';
import { RuleExecutionStatus } from '../../../../../common/detection_engine/schemas/common/schemas';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { SecuritySolutionPluginRouter } from '../../../../types';
import { DETECTION_ENGINE_INDEX_URL } from '../../../../../common/constants';

import { buildSiemResponse } from '../utils';
import { RuleDataPluginService } from '../../../../../../rule_registry/server';
import type { RuleDataPluginService } from '../../../../../../rule_registry/server';
import { fieldAliasesOutdated } from './check_template_version';
import { getIndexVersion } from './get_index_version';
import { isOutdated } from '../../migrations/helpers';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import {
getBasicNoShardsSearchResponse,
} from '../__mocks__/request_responses';
import { configMock, requestContextMock, serverMock } from '../__mocks__';
import { AddPrepackagedRulesSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/add_prepackaged_rules_schema';
import type { AddPrepackagedRulesSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/add_prepackaged_rules_schema';
import { addPrepackedRulesRoute, createPrepackagedRules } from './add_prepackaged_rules_route';
import { listMock } from '../../../../../../lists/server/mocks';
import { ExceptionListClient } from '../../../../../../lists/server';
import type { ExceptionListClient } from '../../../../../../lists/server';
import { installPrepackagedTimelines } from '../../../timeline/routes/prepackaged_timelines/install_prepackaged_timelines';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ import { getExistingPrepackagedRules } from '../../rules/get_existing_prepackage
import { ruleAssetSavedObjectsClientFactory } from '../../rules/rule_asset/rule_asset_saved_objects_client';

import { buildSiemResponse } from '../utils';
import { RulesClient } from '../../../../../../alerting/server';
import type { RulesClient } from '../../../../../../alerting/server';

import { ExceptionListClient } from '../../../../../../lists/server';
import type { ExceptionListClient } from '../../../../../../lists/server';
import { installPrepackagedTimelines } from '../../../timeline/routes/prepackaged_timelines/install_prepackaged_timelines';

export const addPrepackedRulesRoute = (router: SecuritySolutionPluginRouter) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
DETECTION_ENGINE_RULES_URL,
NOTIFICATION_THROTTLE_NO_ACTIONS,
} from '../../../../../common/constants';
import { SetupPlugins } from '../../../../plugin';
import type { SetupPlugins } from '../../../../plugin';
import { buildMlAuthz } from '../../../machine_learning/authz';
import { throwHttpError } from '../../../machine_learning/validation';
import { readRules } from '../../rules/read_rules';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
DETECTION_ENGINE_RULES_URL,
NOTIFICATION_THROTTLE_NO_ACTIONS,
} from '../../../../../common/constants';
import { SetupPlugins } from '../../../../plugin';
import type { SetupPlugins } from '../../../../plugin';
import type { SecuritySolutionPluginRouter } from '../../../../types';
import { buildMlAuthz } from '../../../machine_learning/authz';
import { throwHttpError } from '../../../machine_learning/validation';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { transformError } from '@kbn/securitysolution-es-utils';
import { Logger } from 'src/core/server';
import type { Logger } from 'src/core/server';
import {
exportRulesQuerySchema,
ExportRulesQuerySchemaDecoded,
Expand All @@ -16,7 +16,7 @@ import {
import { buildRouteValidation } from '../../../../utils/build_validation/route_validation';
import type { SecuritySolutionPluginRouter } from '../../../../types';
import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants';
import { ConfigType } from '../../../../config';
import type { ConfigType } from '../../../../config';
import { getNonPackagedRulesCount } from '../../rules/get_existing_prepackaged_rules';
import { getExportByObjectIds } from '../../rules/get_export_by_object_ids';
import { getExportAll } from '../../rules/get_export_all';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
} from '../__mocks__/request_responses';
import { serverMock, requestContextMock, requestMock } from '../__mocks__';
import { findRuleStatusInternalRoute } from './find_rule_status_internal_route';
import { RuleStatusResponse } from '../../rules/types';
import type { RuleStatusResponse } from '../../rules/types';
import { AlertExecutionStatusErrorReasons } from '../../../../../../alerting/common';
import { getQueryRuleParams } from '../../schemas/rule_schemas.mock';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { transformError } from '@kbn/securitysolution-es-utils';
import { Logger } from 'src/core/server';
import type { Logger } from 'src/core/server';
import { findRuleValidateTypeDependents } from '../../../../../common/detection_engine/schemas/request/find_rules_type_dependents';
import {
findRulesSchema,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
} from '../__mocks__/request_responses';
import { serverMock, requestContextMock, requestMock } from '../__mocks__';
import { findRulesStatusesRoute } from './find_rules_status_route';
import { RuleStatusResponse } from '../../rules/types';
import type { RuleStatusResponse } from '../../rules/types';
import { AlertExecutionStatusErrorReasons } from '../../../../../../alerting/common';
import { getQueryRuleParams } from '../../schemas/rule_schemas.mock';

Expand Down
Loading