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

[TGrid] Alerts status update use RBAC api #108092

Merged
merged 25 commits into from
Aug 14, 2021
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
49ebd28
use rac alerts bulk_update
semd Aug 10, 2021
74e64b1
cleanup
semd Aug 10, 2021
73b97e3
adds replace ALERT_STATUS with ALERT_WORKFLOW_STATUS and updates test…
dhurley14 Aug 11, 2021
d6094df
allow object and string types in query param, fixed single update api…
dhurley14 Aug 11, 2021
1a7cfd4
adds additional integration test for when query is a DSL object in ad…
dhurley14 Aug 11, 2021
ec8ada3
merge master
semd Aug 12, 2021
f82720b
Merge branch 'tgrid-bulk-actions-rbac-update' into fix-bulk-update-api
semd Aug 12, 2021
18e1f8b
Merge pull request #1 from dhurley14/fix-bulk-update-api
semd Aug 12, 2021
f91de3f
Merge remote-tracking branch 'upstream/master' into tgrid-bulk-action…
semd Aug 12, 2021
cae0960
optionally use fields api in requests if _source does not contain aut…
dhurley14 Aug 12, 2021
918c4bd
integrate bulk update to all hook calls
semd Aug 12, 2021
7cbec10
adds fields support, fixes bug where we were writing to 'signals.stat…
dhurley14 Aug 12, 2021
dab6006
Merge pull request #2 from dhurley14/sergi_hotfix_bulk_update
semd Aug 12, 2021
120029e
Merge branch 'tgrid-bulk-actions-rbac-update' of https://github.com/s…
semd Aug 12, 2021
7ef80cf
clean up and fixes
semd Aug 12, 2021
df080e2
fix a bug where we were not waiting for updates to complete when usin…
dhurley14 Aug 12, 2021
1710bf3
take index name from ecsData props
semd Aug 13, 2021
22a85ea
Merge remote-tracking branch 'upstream/master' into tgrid-bulk-action…
semd Aug 13, 2021
8138fb6
pr suggestions
semd Aug 13, 2021
23e77f3
some more type fixes
semd Aug 13, 2021
ca20b19
Merge pull request #3 from dhurley14/bulk_update_sec_sol_integration_…
semd Aug 13, 2021
63e033c
Merge remote-tracking branch 'upstream/master' into tgrid-bulk-action…
semd Aug 13, 2021
d5c29fa
refactor and type fixes
semd Aug 13, 2021
5a09258
snapshot updated
semd Aug 13, 2021
36680b4
add alertsConsumers back into tgrid props
dhurley14 Aug 13, 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
2 changes: 1 addition & 1 deletion packages/kbn-rule-data-utils/src/alerts_as_data_rbac.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const AlertConsumers = {
SYNTHETICS: 'synthetics',
} as const;
export type AlertConsumers = typeof AlertConsumers[keyof typeof AlertConsumers];
export type STATUS_VALUES = 'open' | 'acknowledged' | 'closed';
export type STATUS_VALUES = 'open' | 'acknowledged' | 'closed' | 'in-progress'; // TODO: remove 'in-progress' after migration to 'acknowledged'
Copy link
Contributor

Choose a reason for hiding this comment

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

@dplumlee - just pinging you here since you're working on these changes


export const mapConsumerToIndexName: Record<AlertConsumers, string | string[]> = {
apm: '.alerts-observability-apm',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { Logger, ElasticsearchClient, EcsEventOutcome } from '../../../../../src
import { alertAuditEvent, operationAlertAuditActionMap } from './audit_events';
import { AuditLogger } from '../../../security/server';
import {
ALERT_STATUS,
ALERT_WORKFLOW_STATUS,
ALERT_RULE_CONSUMER,
ALERT_RULE_TYPE_ID,
SPACE_IDS,
Expand All @@ -55,11 +55,14 @@ type AlertType = NonNullableProps<
typeof ALERT_RULE_TYPE_ID | typeof ALERT_RULE_CONSUMER | typeof SPACE_IDS
>;

const isValidAlert = (source?: ParsedTechnicalFields): source is AlertType => {
const isValidAlert = (source?: any): source is AlertType => {
semd marked this conversation as resolved.
Show resolved Hide resolved
return (
source?.[ALERT_RULE_TYPE_ID] != null &&
source?.[ALERT_RULE_CONSUMER] != null &&
source?.[SPACE_IDS] != null
(source?._source?.[ALERT_RULE_TYPE_ID] != null &&
source?._source?.[ALERT_RULE_CONSUMER] != null &&
source?._source?.[SPACE_IDS] != null) ||
(source?.fields?.[ALERT_RULE_TYPE_ID][0] != null &&
source?.fields?.[ALERT_RULE_CONSUMER][0] != null &&
source?.fields?.[SPACE_IDS][0] != null)
);
};
export interface ConstructorOptions {
Expand All @@ -80,7 +83,7 @@ export interface BulkUpdateOptions<Params extends AlertTypeParams> {
ids: string[] | undefined | null;
status: STATUS_VALUES;
index: string;
query: string | undefined | null;
query: object | string | undefined | null;
}

interface GetAlertParams {
Expand All @@ -90,7 +93,7 @@ interface GetAlertParams {

interface SingleSearchAfterAndAudit {
id: string | null | undefined;
query: string | null | undefined;
query: object | string | null | undefined;
index?: string;
operation: WriteOperations.Update | ReadOperations.Find | ReadOperations.Get;
lastSortIds: Array<string | number> | undefined;
Expand Down Expand Up @@ -218,6 +221,7 @@ export class AlertsClient {
const config = getEsQueryConfig();

let queryBody = {
fields: [ALERT_RULE_TYPE_ID, ALERT_RULE_CONSUMER, ALERT_WORKFLOW_STATUS, SPACE_IDS],
query: await this.buildEsQueryWithAuthz(query, id, alertSpaceId, operation, config),
sort: [
{
Expand Down Expand Up @@ -245,7 +249,7 @@ export class AlertsClient {
seq_no_primary_term: true,
});

if (!result?.body.hits.hits.every((hit) => isValidAlert(hit._source))) {
if (!result?.body.hits.hits.every((hit) => isValidAlert(hit))) {
const errorMessage = `Invalid alert found with id of "${id}" or with query "${query}" and operation ${operation}`;
this.logger.error(errorMessage);
throw Boom.badData(errorMessage);
Expand Down Expand Up @@ -307,17 +311,25 @@ export class AlertsClient {
);
}

const bulkUpdateRequest = mgetRes.body.docs.flatMap((item) => [
{
update: {
_index: item._index,
_id: item._id,
const bulkUpdateRequest = mgetRes.body.docs.flatMap((item) => {
const fieldToUpdate =
item?._source?.[ALERT_WORKFLOW_STATUS] == null
? { signal: { status } }
: { [ALERT_WORKFLOW_STATUS]: status };
return [
{
update: {
_index: item._index,
_id: item._id,
},
},
},
{
doc: { [ALERT_STATUS]: status },
},
]);
{
doc: {
...fieldToUpdate,
},
},
];
});

const bulkUpdateResponse = await this.esClient.bulk({
body: bulkUpdateRequest,
Expand All @@ -330,7 +342,7 @@ export class AlertsClient {
}

private async buildEsQueryWithAuthz(
query: string | null | undefined,
query: object | string | null | undefined,
id: string | null | undefined,
alertSpaceId: string,
operation: WriteOperations.Update | ReadOperations.Get | ReadOperations.Find,
Expand All @@ -345,15 +357,33 @@ export class AlertsClient {
},
operation
);
return buildEsQuery(
let esQuery;
if (id != null) {
esQuery = { query: `_id:${id}`, language: 'kuery' };
Copy link
Contributor

Choose a reason for hiding this comment

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

It could be helpful to have a comment here describing how these different scenarios happen

} else if (typeof query === 'string') {
esQuery = { query, language: 'kuery' };
} else if (query != null && typeof query === 'object') {
esQuery = [];
}
const builtQuery = buildEsQuery(
undefined,
{ query: query == null ? `_id:${id}` : query, language: 'kuery' },
esQuery == null ? { query: ``, language: 'kuery' } : esQuery,
[
(authzFilter as unknown) as Filter,
({ term: { [SPACE_IDS]: alertSpaceId } } as unknown) as Filter,
],
config
);
if (query != null && typeof query === 'object') {
return {
...builtQuery,
bool: {
...builtQuery.bool,
must: [...builtQuery.bool.must, query],
},
};
}
return builtQuery;
} catch (exc) {
this.logger.error(exc);
throw Boom.expectationFailed(
Expand All @@ -373,7 +403,7 @@ export class AlertsClient {
operation,
}: {
index: string;
query: string;
query: object | string;
operation: WriteOperations.Update | ReadOperations.Find | ReadOperations.Get;
}) {
let lastSortIds;
Expand Down Expand Up @@ -436,7 +466,7 @@ export class AlertsClient {
// first search for the alert by id, then use the alert info to check if user has access to it
const alert = await this.singleSearchAfterAndAudit({
id,
query: null,
query: undefined,
index,
operation: ReadOperations.Get,
lastSortIds: undefined,
Expand Down Expand Up @@ -477,13 +507,18 @@ export class AlertsClient {
throw Boom.notFound(errorMessage);
}

const fieldToUpdate =
semd marked this conversation as resolved.
Show resolved Hide resolved
alert?.hits.hits[0]._source?.[ALERT_WORKFLOW_STATUS] == null
? { signal: { status } }
: { [ALERT_WORKFLOW_STATUS]: status };

const { body: response } = await this.esClient.update<ParsedTechnicalFields>({
...decodeVersion(_version),
id,
index,
body: {
doc: {
[ALERT_STATUS]: status,
...fieldToUpdate,
},
},
refresh: 'wait_for',
Expand Down Expand Up @@ -535,11 +570,11 @@ export class AlertsClient {
refresh: true,
body: {
script: {
source: `if (ctx._source['${ALERT_STATUS}'] != null) {
ctx._source['${ALERT_STATUS}'] = '${status}'
source: `if (ctx._source['${ALERT_WORKFLOW_STATUS}'] != null) {
ctx._source['${ALERT_WORKFLOW_STATUS}'] = '${status}'
}
if (ctx._source['signal.status'] != null) {
ctx._source['signal.status'] = '${status}'
if (ctx._source.signal != null && ctx._source.signal.status != null) {
ctx._source.signal.status = '${status}'
}`,
lang: 'painless',
} as InlineScript,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@ describe('get()', () => {
Array [
Object {
"body": Object {
"fields": Array [
"kibana.alert.rule.rule_type_id",
"kibana.alert.rule.consumer",
"kibana.alert.workflow_status",
"kibana.space_ids",
],
"query": Object {
"bool": Object {
"filter": Array [
Expand Down Expand Up @@ -254,7 +260,7 @@ describe('get()', () => {

await expect(alertsClient.get({ id: fakeAlertId, index: '.alerts-observability-apm' })).rejects
.toThrowErrorMatchingInlineSnapshot(`
"Unable to retrieve alert details for alert with id of \\"myfakeid1\\" or with query \\"null\\" and operation get
"Unable to retrieve alert details for alert with id of \\"myfakeid1\\" or with query \\"undefined\\" and operation get
Error: Error: Unauthorized for fake.rule and apm"
`);

Expand All @@ -281,7 +287,7 @@ describe('get()', () => {
await expect(
alertsClient.get({ id: 'NoxgpHkBqbdrfX07MqXV', index: '.alerts-observability-apm' })
).rejects.toThrowErrorMatchingInlineSnapshot(`
"Unable to retrieve alert details for alert with id of \\"NoxgpHkBqbdrfX07MqXV\\" or with query \\"null\\" and operation get
"Unable to retrieve alert details for alert with id of \\"NoxgpHkBqbdrfX07MqXV\\" or with query \\"undefined\\" and operation get
Error: Error: something went wrong"
`);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import {
ALERT_RULE_CONSUMER,
ALERT_STATUS,
ALERT_WORKFLOW_STATUS,
SPACE_IDS,
ALERT_RULE_TYPE_ID,
} from '@kbn/rule-data-utils';
Expand Down Expand Up @@ -89,8 +89,8 @@ describe('update()', () => {
_source: {
[ALERT_RULE_TYPE_ID]: 'apm.error_rate',
message: 'hello world 1',
[ALERT_WORKFLOW_STATUS]: 'open',
[ALERT_RULE_CONSUMER]: 'apm',
[ALERT_STATUS]: 'open',
[SPACE_IDS]: [DEFAULT_SPACE],
},
},
Expand Down Expand Up @@ -139,7 +139,7 @@ describe('update()', () => {
Object {
"body": Object {
"doc": Object {
"${ALERT_STATUS}": "closed",
"${ALERT_WORKFLOW_STATUS}": "closed",
},
},
"id": "1",
Expand Down Expand Up @@ -175,8 +175,8 @@ describe('update()', () => {
_source: {
[ALERT_RULE_TYPE_ID]: 'apm.error_rate',
message: 'hello world 1',
[ALERT_WORKFLOW_STATUS]: 'open',
[ALERT_RULE_CONSUMER]: 'apm',
[ALERT_STATUS]: 'open',
[SPACE_IDS]: [DEFAULT_SPACE],
},
},
Expand Down Expand Up @@ -249,7 +249,7 @@ describe('update()', () => {
_source: {
[ALERT_RULE_TYPE_ID]: fakeRuleTypeId,
[ALERT_RULE_CONSUMER]: 'apm',
[ALERT_STATUS]: 'open',
[ALERT_WORKFLOW_STATUS]: 'open',
[SPACE_IDS]: [DEFAULT_SPACE],
},
},
Expand Down Expand Up @@ -330,8 +330,8 @@ describe('update()', () => {
_source: {
[ALERT_RULE_TYPE_ID]: 'apm.error_rate',
message: 'hello world 1',
[ALERT_WORKFLOW_STATUS]: 'open',
[ALERT_RULE_CONSUMER]: 'apm',
[ALERT_STATUS]: 'open',
[SPACE_IDS]: [DEFAULT_SPACE],
},
},
Expand Down Expand Up @@ -391,7 +391,7 @@ describe('update()', () => {
[ALERT_RULE_TYPE_ID]: 'apm.error_rate',
message: 'hello world 1',
[ALERT_RULE_CONSUMER]: 'apm',
[ALERT_STATUS]: 'open',
[ALERT_WORKFLOW_STATUS]: 'open',
[SPACE_IDS]: [DEFAULT_SPACE],
},
},
Expand Down
16 changes: 13 additions & 3 deletions x-pack/plugins/rule_registry/server/routes/bulk_update_alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,26 @@ export const bulkUpdateAlertsRoute = (router: IRouter<RacRequestHandlerContext>)
body: buildRouteValidation(
t.union([
t.strict({
status: t.union([t.literal('open'), t.literal('closed')]),
status: t.union([
t.literal('open'),
t.literal('closed'),
t.literal('in-progress'), // TODO: remove after migration to acknowledged
Copy link
Contributor

Choose a reason for hiding this comment

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

t.literal('acknowledged'),
]),
index: t.string,
ids: t.array(t.string),
query: t.undefined,
}),
t.strict({
status: t.union([t.literal('open'), t.literal('closed')]),
status: t.union([
t.literal('open'),
t.literal('closed'),
t.literal('in-progress'), // TODO: remove after migration to acknowledged
Copy link
Contributor

Choose a reason for hiding this comment

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

t.literal('acknowledged'),
]),
index: t.string,
ids: t.undefined,
query: t.string,
query: t.union([t.object, t.string]),
}),
])
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ const AlertContextMenuComponent: React.FC<AlertContextMenuProps> = ({
timelineId,
}) => {
const [isPopoverOpen, setPopover] = useState(false);

const ruleId = get(0, ecsRowData?.signal?.rule?.id);
const ruleName = get(0, ecsRowData?.signal?.rule?.name);

Expand Down Expand Up @@ -116,6 +115,7 @@ const AlertContextMenuComponent: React.FC<AlertContextMenuProps> = ({
const { actionItems } = useAlertsActions({
alertStatus,
eventId: ecsRowData?._id,
indexName: ecsRowData?._index ?? '',
timelineId,
closePopover,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,16 @@ interface Props {
closePopover: () => void;
eventId: string;
timelineId: string;
indexName: string;
}

export const useAlertsActions = ({ alertStatus, closePopover, eventId, timelineId }: Props) => {
export const useAlertsActions = ({
alertStatus,
closePopover,
eventId,
timelineId,
indexName,
}: Props) => {
const dispatch = useDispatch();
const [, dispatchToaster] = useStateToaster();

Expand Down Expand Up @@ -100,6 +107,7 @@ export const useAlertsActions = ({ alertStatus, closePopover, eventId, timelineI
const actionItems = useStatusBulkActionItems({
eventIds: [eventId],
currentStatus: alertStatus,
indexName,
setEventsLoading,
setEventsDeleted,
onUpdateSuccess: onAlertStatusUpdateSuccess,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const TakeActionDropdown = React.memo(
onAddExceptionTypeClick,
onAddIsolationStatusClick,
refetch,
indexName,
timelineId,
}: {
detailsData: TimelineEventsDetailsItem[] | null;
Expand All @@ -57,6 +58,7 @@ export const TakeActionDropdown = React.memo(
loadingEventDetails: boolean;
nonEcsData?: TimelineNonEcsData[];
refetch: (() => void) | undefined;
indexName: string;
onAddEventFilterClick: () => void;
onAddExceptionTypeClick: (type: ExceptionListType) => void;
onAddIsolationStatusClick: (action: 'isolateHost' | 'unisolateHost') => void;
Expand Down Expand Up @@ -154,6 +156,7 @@ export const TakeActionDropdown = React.memo(
const { actionItems } = useAlertsActions({
alertStatus: actionsData.alertStatus,
eventId: actionsData.eventId,
indexName,
timelineId,
closePopover: closePopoverAndFlyout,
});
Expand Down
Loading