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 Solution] [Detections] Create a 'partial failure' status for rules #84293

Merged
merged 2 commits into from
Dec 1, 2020
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,12 @@ export type SeverityMappingOrUndefined = t.TypeOf<typeof severityMappingOrUndefi
export const status = t.keyof({ open: null, closed: null, 'in-progress': null });
export type Status = t.TypeOf<typeof status>;

export const job_status = t.keyof({ succeeded: null, failed: null, 'going to run': null });
export const job_status = t.keyof({
succeeded: null,
failed: null,
'going to run': null,
'partial failure': null,
});
export type JobStatus = t.TypeOf<typeof job_status>;

export const conflicts = t.keyof({ abort: null, proceed: null });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ export const getStatusColor = (status: RuleStatusType | string | null) =>
? 'success'
: status === 'failed'
? 'danger'
: status === 'executing' || status === 'going to run'
: status === 'executing' || status === 'going to run' || status === 'partial failure'
? 'warning'
: 'subdued';
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ const MetaRule = t.intersection([
}),
]);

const StatusTypes = t.union([
t.literal('succeeded'),
t.literal('failed'),
t.literal('going to run'),
t.literal('partial failure'),
]);

export const RuleSchema = t.intersection([
t.type({
author,
Expand Down Expand Up @@ -108,13 +115,15 @@ export const RuleSchema = t.intersection([
license,
last_failure_at: t.string,
last_failure_message: t.string,
last_success_message: t.string,
last_success_at: t.string,
meta: MetaRule,
machine_learning_job_id: t.string,
output_index: t.string,
query: t.string,
rule_name_override,
saved_id: t.string,
status: t.string,
status: StatusTypes,
status_date: t.string,
threshold,
threat_query,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,18 +275,33 @@ export const RuleDetailsPageComponent: FC<PropsFromRedux> = ({
),
[ruleDetailTabs, ruleDetailTab, setRuleDetailTab]
);
const ruleError = useMemo(
() =>
const ruleError = useMemo(() => {
if (
rule?.status === 'failed' &&
ruleDetailTab === RuleDetailTabs.alerts &&
rule?.last_failure_at != null ? (
rule?.last_failure_at != null
) {
return (
<RuleStatusFailedCallOut
message={rule?.last_failure_message ?? ''}
date={rule?.last_failure_at}
/>
) : null,
[rule, ruleDetailTab]
);
);
} else if (
rule?.status === 'partial failure' &&
ruleDetailTab === RuleDetailTabs.alerts &&
rule?.last_success_at != null
) {
return (
<RuleStatusFailedCallOut
message={rule?.last_success_message ?? ''}
date={rule?.last_success_at}
color="warning"
/>
);
}
return null;
}, [rule, ruleDetailTab]);

const updateDateRangeCallback = useCallback<UpdateDateRange>(
({ x }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ describe('RuleStatusFailedCallOut', () => {
it('renders correctly', () => {
const wrapper = shallow(<RuleStatusFailedCallOut date="date" message="message" />);

expect(wrapper.find('EuiCallOut')).toHaveLength(1);
});
it('renders correctly with optional params', () => {
const wrapper = shallow(
<RuleStatusFailedCallOut date="date" message="message" color="warning" />
);

expect(wrapper.find('EuiCallOut')).toHaveLength(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,26 @@ import * as i18n from './translations';
interface RuleStatusFailedCallOutComponentProps {
date: string;
message: string;
color?: 'danger' | 'primary' | 'success' | 'warning';
}

const RuleStatusFailedCallOutComponent: React.FC<RuleStatusFailedCallOutComponentProps> = ({
date,
message,
color,
}) => (
<EuiCallOut
title={
<EuiFlexGroup gutterSize="xs" alignItems="center" justifyContent="flexStart">
<EuiFlexItem grow={false}>{i18n.ERROR_CALLOUT_TITLE}</EuiFlexItem>
<EuiFlexItem grow={false}>
{color === 'warning' ? i18n.PARTIAL_FAILURE_CALLOUT_TITLE : i18n.ERROR_CALLOUT_TITLE}
</EuiFlexItem>
<EuiFlexItem grow={true}>
<FormattedDate value={date} fieldName="last_failure_at" />
</EuiFlexItem>
</EuiFlexGroup>
}
color="danger"
color={color ? color : 'danger'}
iconType="alert"
>
<p>{message}</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ export const ERROR_CALLOUT_TITLE = i18n.translate(
}
);

export const PARTIAL_FAILURE_CALLOUT_TITLE = i18n.translate(
'xpack.securitySolution.detectionEngine.ruleDetails.partialErrorCalloutTitle',
{
defaultMessage: 'Partial rule failure at',
}
);

export const FAILURE_HISTORY_TAB = i18n.translate(
'xpack.securitySolution.detectionEngine.ruleDetails.failureHistoryTab',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export const ruleStatusServiceFactoryMock = async ({

success: jest.fn(),

partialFailure: jest.fn(),

error: jest.fn(),
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,21 @@ describe('buildRuleStatusAttributes', () => {
expect(result.statusDate).toEqual(result.lastSuccessAt);
});

it('returns partial failure fields if "partial failure"', () => {
const result = buildRuleStatusAttributes(
'partial failure',
'some indices missing timestamp override field'
);
expect(result).toEqual({
status: 'partial failure',
statusDate: expectIsoDateString,
lastSuccessAt: expectIsoDateString,
lastSuccessMessage: 'some indices missing timestamp override field',
});

expect(result.statusDate).toEqual(result.lastSuccessAt);
});

it('returns failure fields if "failed"', () => {
const result = buildRuleStatusAttributes('failed', 'failure message');
expect(result).toEqual({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ interface Attributes {
export interface RuleStatusService {
goingToRun: () => Promise<void>;
success: (message: string, attributes?: Attributes) => Promise<void>;
partialFailure: (message: string, attributes?: Attributes) => Promise<void>;
error: (message: string, attributes?: Attributes) => Promise<void>;
}

Expand All @@ -46,6 +47,13 @@ export const buildRuleStatusAttributes: (
lastSuccessMessage: message,
};
}
case 'partial failure': {
return {
...baseAttributes,
lastSuccessAt: now,
lastSuccessMessage: message,
};
}
case 'failed': {
return {
...baseAttributes,
Expand Down Expand Up @@ -93,6 +101,18 @@ export const ruleStatusServiceFactory = async ({
});
},

partialFailure: async (message, attributes) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the plan to use this function in signal_rule_alert_type.ts or is it already being used somewhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, there's no place in the code where this function is being used. When this gets merged I will be incorporating it into the multiple timestamps pr #83134 I think by returning an error type instead of relying on the result.success boolean to write an error / success status.

const [currentStatus] = await getOrCreateRuleStatuses({
alertId,
ruleStatusClient,
});

await ruleStatusClient.update(currentStatus.id, {
...currentStatus.attributes,
...buildRuleStatusAttributes('partial failure', message, attributes),
});
},

error: async (message, attributes) => {
const ruleStatuses = await getOrCreateRuleStatuses({
alertId,
Expand Down