[8.8] [Security Solutions] Error pops up on rules creation for uninstalled ML jobs (#159316) (#160043)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Security Solutions] Error pops up on rules creation for uninstalled
ML jobs (#159316)](https://github.com/elastic/kibana/pull/159316)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ievgen
Sorokopud","email":"ievgen.sorokopud@elastic.co"},"sourceCommit":{"committedDate":"2023-06-19T11:11:19Z","message":"[Security
Solutions] Error pops up on rules creation for uninstalled ML jobs
(#159316)\n\n## Summary\r\n\r\nOriginal ticket:
https://github.com/elastic/kibana/issues/151168\r\n\r\nThese changes fix
the issue with unexpected error toasts being shown on\r\nselecting
uninstalled ML jobs while creating an ML rule.\r\n\r\nThe issue was that
we used ML API `'/internal/ml/jobs/jobs'` to fetch\r\nthe information
about the job which assumed that requested jobs are\r\ninstalled. When
jobs are not, it throws an
exception.\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"f0f299f5c1fb649ca71d6aa85d468dd9f5575d50","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:
SecuritySolution","ci:cloud-deploy","8.9
candidate","v8.9.0","Team:Detection
Engine","v8.8.2"],"number":159316,"url":"https://github.com/elastic/kibana/pull/159316","mergeCommit":{"message":"[Security
Solutions] Error pops up on rules creation for uninstalled ML jobs
(#159316)\n\n## Summary\r\n\r\nOriginal ticket:
https://github.com/elastic/kibana/issues/151168\r\n\r\nThese changes fix
the issue with unexpected error toasts being shown on\r\nselecting
uninstalled ML jobs while creating an ML rule.\r\n\r\nThe issue was that
we used ML API `'/internal/ml/jobs/jobs'` to fetch\r\nthe information
about the job which assumed that requested jobs are\r\ninstalled. When
jobs are not, it throws an
exception.\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"f0f299f5c1fb649ca71d6aa85d468dd9f5575d50"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/159316","number":159316,"mergeCommit":{"message":"[Security
Solutions] Error pops up on rules creation for uninstalled ML jobs
(#159316)\n\n## Summary\r\n\r\nOriginal ticket:
https://github.com/elastic/kibana/issues/151168\r\n\r\nThese changes fix
the issue with unexpected error toasts being shown on\r\nselecting
uninstalled ML jobs while creating an ML rule.\r\n\r\nThe issue was that
we used ML API `'/internal/ml/jobs/jobs'` to fetch\r\nthe information
about the job which assumed that requested jobs are\r\ninstalled. When
jobs are not, it throws an
exception.\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"f0f299f5c1fb649ca71d6aa85d468dd9f5575d50"}},{"branch":"8.8","label":"v8.8.2","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Ievgen Sorokopud 2023-06-20 22:03:15 +02:00 committed by GitHub
parent 61ad0bc1b4
commit c3cb2f713a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 231 additions and 90 deletions

View file

@ -12,11 +12,11 @@ import type { FieldSpec, DataViewSpec } from '@kbn/data-views-plugin/common';
import { useAppToasts } from '../../../common/hooks/use_app_toasts';
import type { Rule } from '../../rule_management/logic/types';
import { useGetInstalledJob } from '../../../common/components/ml/hooks/use_get_jobs';
import { useKibana } from '../../../common/lib/kibana';
import { useFetchIndex } from '../../../common/containers/source';
import * as i18n from '../../../common/containers/source/translations';
import { useRuleIndices } from '../../rule_management/logic/use_rule_indices';
export interface ReturnUseFetchExceptionFlyoutData {
isLoading: boolean;
@ -72,20 +72,20 @@ export const useFetchIndexPatterns = (rules: Rule[] | null): ReturnUseFetchExcep
() => (isMLRule && isSingleRule && rules != null ? rules[0].machine_learning_job_id ?? [] : []),
[isMLRule, isSingleRule, rules]
);
const { loading: mlJobLoading, jobs } = useGetInstalledJob(memoMlJobIds);
const { mlJobLoading, ruleIndices: mlRuleIndices } = useRuleIndices(memoMlJobIds);
// We only want to provide a non empty array if it's an ML rule and we were able to fetch
// the index patterns, or if it's a rule not using data views. Otherwise, return an empty
// empty array to avoid making the `useFetchIndex` call
const memoRuleIndices = useMemo(() => {
if (isMLRule && jobs.length > 0) {
return jobs[0].results_index_name ? [`.ml-anomalies-${jobs[0].results_index_name}`] : [];
if (isMLRule && mlRuleIndices.length > 0) {
return mlRuleIndices;
} else if (memoDataViewId != null) {
return [];
} else {
return memoNonDataViewIndexPatterns;
}
}, [jobs, isMLRule, memoDataViewId, memoNonDataViewIndexPatterns]);
}, [isMLRule, memoDataViewId, memoNonDataViewIndexPatterns, mlRuleIndices]);
const [
isIndexPatternLoading,

View file

@ -9,8 +9,10 @@ import { renderHook } from '@testing-library/react-hooks';
import { useRuleIndices } from './use_rule_indices';
import { useGetInstalledJob } from '../../../common/components/ml/hooks/use_get_jobs';
import { useSecurityJobs } from '../../../common/components/ml_popover/hooks/use_security_jobs';
jest.mock('../../../common/components/ml/hooks/use_get_jobs');
jest.mock('../../../common/components/ml_popover/hooks/use_security_jobs');
describe('useRuleIndices', () => {
beforeEach(() => {
@ -18,6 +20,10 @@ describe('useRuleIndices', () => {
});
it('should return handle undefined parameters', async () => {
(useSecurityJobs as jest.Mock).mockImplementation(() => ({
jobs: [{ id: 'job1' }, { id: 'job2' }],
loading: false,
}));
(useGetInstalledJob as jest.Mock).mockImplementation((jobIds: string[]) => {
expect(jobIds).toEqual([]);
return { loading: false, jobs: [] };
@ -30,6 +36,10 @@ describe('useRuleIndices', () => {
});
it('should return default indices if ML job is not specified', async () => {
(useSecurityJobs as jest.Mock).mockImplementation(() => ({
jobs: [{ id: 'job1' }, { id: 'job2' }],
loading: false,
}));
(useGetInstalledJob as jest.Mock).mockImplementation((jobIds: string[]) => {
expect(jobIds).toEqual([]);
return { loading: false, jobs: [] };
@ -44,6 +54,13 @@ describe('useRuleIndices', () => {
it('should return default indices if ML job is not specified 1', async () => {
const machineLearningJobId = ['ml-job-1', 'ml-job-2'];
(useSecurityJobs as jest.Mock).mockImplementation(() => ({
jobs: [
{ id: 'ml-job-1', isInstalled: true },
{ id: 'ml-job-2', isInstalled: true },
],
loading: false,
}));
(useGetInstalledJob as jest.Mock).mockImplementation((jobIds: string[]) => {
expect(jobIds).toEqual(machineLearningJobId);
return {
@ -55,7 +72,55 @@ describe('useRuleIndices', () => {
const { result } = renderHook(() => useRuleIndices(machineLearningJobId, defaultIndices));
expect(result.current).toEqual({
mlJobLoading: false,
ruleIndices: ['.ml-anomalies-index1'],
ruleIndices: ['.ml-anomalies-index1', '.ml-anomalies-index2'],
});
});
it('should return indices of installed jobs only', async () => {
const machineLearningJobId = ['ml-job-1', 'ml-job-2'];
(useSecurityJobs as jest.Mock).mockImplementation(() => ({
jobs: [
{ id: 'ml-job-1', isInstalled: false },
{ id: 'ml-job-2', isInstalled: true },
],
loading: false,
}));
(useGetInstalledJob as jest.Mock).mockImplementation((jobIds: string[]) => {
expect(jobIds).toEqual(['ml-job-2']);
return {
loading: false,
jobs: [{ results_index_name: 'index2' }],
};
});
const defaultIndices = ['index1', 'index2'];
const { result } = renderHook(() => useRuleIndices(machineLearningJobId, defaultIndices));
expect(result.current).toEqual({
mlJobLoading: false,
ruleIndices: ['.ml-anomalies-index2'],
});
});
it('should return default indices if ML jobs are not installed', async () => {
const machineLearningJobId = ['ml-job-1', 'ml-job-2'];
(useSecurityJobs as jest.Mock).mockImplementation(() => ({
jobs: [
{ id: 'ml-job-1', isInstalled: false },
{ id: 'ml-job-2', isInstalled: false },
],
loading: false,
}));
(useGetInstalledJob as jest.Mock).mockImplementation((jobIds: string[]) => {
expect(jobIds).toEqual([]);
return {
loading: false,
jobs: [],
};
});
const defaultIndices = ['index1', 'index2'];
const { result } = renderHook(() => useRuleIndices(machineLearningJobId, defaultIndices));
expect(result.current).toEqual({
mlJobLoading: false,
ruleIndices: defaultIndices,
});
});
});

View file

@ -6,19 +6,39 @@
*/
import { useMemo } from 'react';
import { useSecurityJobs } from '../../../common/components/ml_popover/hooks/use_security_jobs';
import { useGetInstalledJob } from '../../../common/components/ml/hooks/use_get_jobs';
export const useRuleIndices = (machineLearningJobId?: string[], defaultRuleIndices?: string[]) => {
const memoMlJobIds = useMemo(() => machineLearningJobId ?? [], [machineLearningJobId]);
const { loading: mlJobLoading, jobs } = useGetInstalledJob(memoMlJobIds);
const { loading: mlSecurityJobLoading, jobs } = useSecurityJobs();
const memoSelectedMlJobs = useMemo(
() => jobs.filter(({ id }) => memoMlJobIds.includes(id)),
[jobs, memoMlJobIds]
);
// Filter jobs that are installed. For those jobs we can get the index pattern from `job.results_index_name` field
const memoInstalledMlJobs = useMemo(
() => memoSelectedMlJobs.filter(({ isInstalled }) => isInstalled).map((j) => j.id),
[memoSelectedMlJobs]
);
const { loading: mlInstalledJobLoading, jobs: installedJobs } =
useGetInstalledJob(memoInstalledMlJobs);
const memoMlIndices = useMemo(() => {
const installedJobsIndices = installedJobs.map((j) => `.ml-anomalies-${j.results_index_name}`);
return [...new Set(installedJobsIndices)];
}, [installedJobs]);
const memoRuleIndices = useMemo(() => {
if (jobs.length > 0) {
return jobs[0].results_index_name ? [`.ml-anomalies-${jobs[0].results_index_name}`] : [];
if (memoMlIndices.length > 0) {
return memoMlIndices;
} else {
return defaultRuleIndices ?? [];
}
}, [jobs, defaultRuleIndices]);
}, [defaultRuleIndices, memoMlIndices]);
return { mlJobLoading, ruleIndices: memoRuleIndices };
return {
mlJobLoading: mlSecurityJobLoading || mlInstalledJobLoading,
ruleIndices: memoRuleIndices,
};
};

View file

@ -13,6 +13,7 @@ import { stubIndexPattern } from '@kbn/data-plugin/common/stubs';
import { StepAboutRule } from '.';
import { useFetchIndex } from '../../../../common/containers/source';
import { useGetInstalledJob } from '../../../../common/components/ml/hooks/use_get_jobs';
import { useSecurityJobs } from '../../../../common/components/ml_popover/hooks/use_security_jobs';
import { mockAboutStepRule } from '../../../../detection_engine/rule_management_ui/components/rules_table/__mocks__/mock';
import { StepRuleDescription } from '../description_step';
import { stepAboutDefaultValue } from './default_value';
@ -29,6 +30,7 @@ import { TestProviders } from '../../../../common/mock';
jest.mock('../../../../common/lib/kibana');
jest.mock('../../../../common/containers/source');
jest.mock('../../../../common/components/ml/hooks/use_get_jobs');
jest.mock('../../../../common/components/ml_popover/hooks/use_security_jobs');
jest.mock('@elastic/eui', () => {
const original = jest.requireActual('@elastic/eui');
return {
@ -43,7 +45,7 @@ jest.mock('@elastic/eui', () => {
export const stepDefineStepMLRule: DefineStepRule = {
ruleType: 'machine_learning',
index: [],
index: ['default-index-*'],
queryBar: { query: { query: '', language: '' }, filters: [], saved_id: null },
machineLearningJobId: ['auth_high_count_logon_events_for_a_source_ip'],
anomalyThreshold: 50,
@ -69,6 +71,7 @@ export const stepDefineStepMLRule: DefineStepRule = {
describe('StepAboutRuleComponent', () => {
let useGetInstalledJobMock: jest.Mock;
let useSecurityJobsMock: jest.Mock;
let formHook: RuleStepsFormHooks[RuleStep.aboutRule] | null = null;
const setFormHook = <K extends keyof RuleStepsFormHooks>(
step: K,
@ -88,6 +91,7 @@ describe('StepAboutRuleComponent', () => {
useGetInstalledJobMock = (useGetInstalledJob as jest.Mock).mockImplementation(() => ({
jobs: [],
}));
useSecurityJobsMock = (useSecurityJobs as jest.Mock).mockImplementation(() => ({ jobs: [] }));
});
it('it renders StepRuleDescription if isReadOnlyView is true and "name" property exists', () => {
@ -393,8 +397,14 @@ describe('StepAboutRuleComponent', () => {
});
});
it('should use index based on ML jobs when editing ML rule', async () => {
it('should use index based on ML jobs when creating/editing ML rule', async () => {
(useFetchIndex as jest.Mock).mockClear();
useSecurityJobsMock.mockImplementation(() => {
return {
jobs: [{ id: 'auth_high_count_logon_events_for_a_source_ip', isInstalled: true }],
loading: false,
};
});
useGetInstalledJobMock.mockImplementation((jobIds: string[]) => {
expect(jobIds).toEqual(['auth_high_count_logon_events_for_a_source_ip']);
return { jobs: [{ results_index_name: 'shared' }] };
@ -418,4 +428,35 @@ describe('StepAboutRuleComponent', () => {
const indexNames = ['.ml-anomalies-shared'];
expect(useFetchIndex).lastCalledWith(indexNames);
});
it('should use default rule index if selected ML jobs are not installed when creating/editing ML rule', async () => {
(useFetchIndex as jest.Mock).mockClear();
useSecurityJobsMock.mockImplementation(() => {
return {
jobs: [{ id: 'auth_high_count_logon_events_for_a_source_ip', isInstalled: false }],
loading: false,
};
});
useGetInstalledJobMock.mockImplementation((jobIds: string[]) => {
expect(jobIds).toEqual([]);
return { jobs: [] };
});
mount(
<StepAboutRule
addPadding={true}
defaultValues={stepAboutDefaultValue}
defineRuleData={stepDefineStepMLRule}
descriptionColumns="multi"
isReadOnlyView={false}
setForm={setFormHook}
isLoading={false}
/>,
{
wrappingComponent: TestProviders,
}
);
expect(useFetchIndex).lastCalledWith(stepDefineStepMLRule.index);
});
});

View file

@ -16,6 +16,7 @@ import { HeaderSection } from '../../../../common/components/header_section';
import { StepAboutRule } from '../step_about_rule';
import type { AboutStepRule } from '../../../pages/detection_engine/rules/types';
import { getMockTheme } from '../../../../common/lib/kibana/kibana_react.mock';
import { TestProviders } from '../../../../common/mock';
jest.mock('../../../../common/lib/kibana');
@ -99,17 +100,19 @@ describe('StepAboutRuleToggleDetails', () => {
describe('note value does exist', () => {
test('it renders toggle buttons, defaulted to "details"', () => {
const wrapper = mount(
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: '',
}}
stepData={mockRule}
/>
</ThemeProvider>
<TestProviders>
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: '',
}}
stepData={mockRule}
/>
</ThemeProvider>
</TestProviders>
);
expect(wrapper.find(EuiButtonGroup).exists()).toBeTruthy();
@ -119,17 +122,19 @@ describe('StepAboutRuleToggleDetails', () => {
test('it allows users to toggle between "details" and "note"', () => {
const wrapper = mount(
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: '',
}}
stepData={mockRule}
/>
</ThemeProvider>
<TestProviders>
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: '',
}}
stepData={mockRule}
/>
</ThemeProvider>
</TestProviders>
);
expect(wrapper.find('[idSelected="details"]').exists()).toBeTruthy();
@ -147,17 +152,19 @@ describe('StepAboutRuleToggleDetails', () => {
test('it displays notes markdown when user toggles to "notes"', () => {
const wrapper = mount(
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: '',
}}
stepData={mockRule}
/>
</ThemeProvider>
<TestProviders>
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: '',
}}
stepData={mockRule}
/>
</ThemeProvider>
</TestProviders>
);
wrapper
@ -176,17 +183,19 @@ describe('StepAboutRuleToggleDetails', () => {
describe('setup value is empty string', () => {
test('it does render toggle buttons if note is not empty', () => {
const wrapper = mount(
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: '',
}}
stepData={mockRule}
/>
</ThemeProvider>
<TestProviders>
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: '',
}}
stepData={mockRule}
/>
</ThemeProvider>
</TestProviders>
);
expect(wrapper.find(EuiButtonGroup).exists()).toBeTruthy();
@ -199,17 +208,19 @@ describe('StepAboutRuleToggleDetails', () => {
describe('setup value does exist', () => {
test('it renders toggle buttons, defaulted to "details"', () => {
const wrapper = mount(
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: mockRule.note, // TODO: Update to mockRule.setup once supported in UI (and mock can be updated)
}}
stepData={mockRule}
/>
</ThemeProvider>
<TestProviders>
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: mockRule.note, // TODO: Update to mockRule.setup once supported in UI (and mock can be updated)
}}
stepData={mockRule}
/>
</ThemeProvider>
</TestProviders>
);
expect(wrapper.find(EuiButtonGroup).exists()).toBeTruthy();
@ -220,17 +231,19 @@ describe('StepAboutRuleToggleDetails', () => {
test('it allows users to toggle between "details" and "setup"', () => {
const wrapper = mount(
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: mockRule.note, // TODO: Update to mockRule.setup once supported in UI (and mock can be updated)
}}
stepData={mockRule}
/>
</ThemeProvider>
<TestProviders>
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: mockRule.note, // TODO: Update to mockRule.setup once supported in UI (and mock can be updated)
}}
stepData={mockRule}
/>
</ThemeProvider>
</TestProviders>
);
expect(wrapper.find('[idSelected="details"]').exists()).toBeTruthy();
@ -250,17 +263,19 @@ describe('StepAboutRuleToggleDetails', () => {
test('it displays notes markdown when user toggles to "setup"', () => {
const wrapper = mount(
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: mockRule.note, // TODO: Update to mockRule.setup once supported in UI (and mock can be updated)
}}
stepData={mockRule}
/>
</ThemeProvider>
<TestProviders>
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: mockRule.note,
description: mockRule.description,
setup: mockRule.note, // TODO: Update to mockRule.setup once supported in UI (and mock can be updated)
}}
stepData={mockRule}
/>
</ThemeProvider>
</TestProviders>
);
wrapper