Skip to content

Commit

Permalink
[Event Log] add rel=primary to saved objects for query targets
Browse files Browse the repository at this point in the history
resolves elastic#62668

Adds a property named `rel` to the nested saved objects in the event
documents, whose value should not be set, or set to `primary`.
The query by saved object function changes to only match event documents
with that saved objects if it has the `rel: primary` value.

This is used to limit searching alerting's executeAction event document
with only the alert saved object, and not the action saved object (this
document has an alert and action saved object). The alert saved object
has the `rel: primary` field set, and the action does not.  Previously,
those documents were returned with a query of the action saved object.
  • Loading branch information
pmuellr committed Apr 28, 2020
1 parent 36b4864 commit b8626b1
Show file tree
Hide file tree
Showing 17 changed files with 135 additions and 12 deletions.
13 changes: 11 additions & 2 deletions x-pack/plugins/actions/server/lib/action_executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
import { EncryptedSavedObjectsPluginStart } from '../../../encrypted_saved_objects/server';
import { SpacesServiceSetup } from '../../../spaces/server';
import { EVENT_LOG_ACTIONS } from '../plugin';
import { IEvent, IEventLogger } from '../../../event_log/server';
import { IEvent, IEventLogger, SAVED_OBJECT_REL_PRIMARY } from '../../../event_log/server';

export interface ActionExecutorContext {
logger: Logger;
Expand Down Expand Up @@ -110,7 +110,16 @@ export class ActionExecutor {
const actionLabel = `${actionTypeId}:${actionId}: ${name}`;
const event: IEvent = {
event: { action: EVENT_LOG_ACTIONS.execute },
kibana: { saved_objects: [{ type: 'action', id: actionId, ...namespace }] },
kibana: {
saved_objects: [
{
rel: SAVED_OBJECT_REL_PRIMARY,
type: 'action',
id: actionId,
...namespace,
},
],
},
};

eventLogger.startTiming(event);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ test('calls actionsPlugin.execute per selected action', async () => {
"saved_objects": Array [
Object {
"id": "1",
"rel": "primary",
"type": "alert",
},
Object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { AlertAction, State, Context, AlertType } from '../types';
import { Logger } from '../../../../../src/core/server';
import { transformActionParams } from './transform_action_params';
import { PluginStartContract as ActionsPluginStartContract } from '../../../../plugins/actions/server';
import { IEventLogger, IEvent } from '../../../event_log/server';
import { IEventLogger, IEvent, SAVED_OBJECT_REL_PRIMARY } from '../../../event_log/server';
import { EVENT_LOG_ACTIONS } from '../plugin';

interface CreateExecutionHandlerOptions {
Expand Down Expand Up @@ -96,7 +96,7 @@ export function createExecutionHandler({
instance_id: alertInstanceId,
},
saved_objects: [
{ type: 'alert', id: alertId, ...namespace },
{ rel: SAVED_OBJECT_REL_PRIMARY, type: 'alert', id: alertId, ...namespace },
{ type: 'action', id: action.id, ...namespace },
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ describe('Task Runner', () => {
Object {
"id": "1",
"namespace": undefined,
"rel": "primary",
"type": "alert",
},
],
Expand Down Expand Up @@ -234,6 +235,7 @@ describe('Task Runner', () => {
Object {
"id": "1",
"namespace": undefined,
"rel": "primary",
"type": "alert",
},
],
Expand All @@ -254,6 +256,7 @@ describe('Task Runner', () => {
Object {
"id": "1",
"namespace": undefined,
"rel": "primary",
"type": "alert",
},
],
Expand All @@ -274,6 +277,7 @@ describe('Task Runner', () => {
Object {
"id": "1",
"namespace": undefined,
"rel": "primary",
"type": "alert",
},
Object {
Expand Down Expand Up @@ -351,6 +355,7 @@ describe('Task Runner', () => {
Object {
"id": "1",
"namespace": undefined,
"rel": "primary",
"type": "alert",
},
],
Expand All @@ -371,6 +376,7 @@ describe('Task Runner', () => {
Object {
"id": "1",
"namespace": undefined,
"rel": "primary",
"type": "alert",
},
],
Expand Down Expand Up @@ -568,6 +574,7 @@ describe('Task Runner', () => {
Object {
"id": "1",
"namespace": undefined,
"rel": "primary",
"type": "alert",
},
],
Expand Down
22 changes: 19 additions & 3 deletions x-pack/plugins/alerting/server/task_runner/task_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { promiseResult, map, Resultable, asOk, asErr, resolveErr } from '../lib/
import { taskInstanceToAlertTaskInstance } from './alert_task_instance';
import { AlertInstances } from '../alert_instance/alert_instance';
import { EVENT_LOG_ACTIONS } from '../plugin';
import { IEvent, IEventLogger } from '../../../event_log/server';
import { IEvent, IEventLogger, SAVED_OBJECT_REL_PRIMARY } from '../../../event_log/server';
import { isAlertSavedObjectNotFoundError } from '../lib/is_alert_not_found_error';

const FALLBACK_RETRY_INTERVAL: IntervalSchedule = { interval: '5m' };
Expand Down Expand Up @@ -174,7 +174,16 @@ export class TaskRunner {
const alertLabel = `${this.alertType.id}:${alertId}: '${name}'`;
const event: IEvent = {
event: { action: EVENT_LOG_ACTIONS.execute },
kibana: { saved_objects: [{ type: 'alert', id: alertId, namespace }] },
kibana: {
saved_objects: [
{
rel: SAVED_OBJECT_REL_PRIMARY,
type: 'alert',
id: alertId,
namespace,
},
],
},
};
eventLogger.startTiming(event);

Expand Down Expand Up @@ -393,7 +402,14 @@ function generateNewAndResolvedInstanceEvents(params: GenerateNewAndResolvedInst
alerting: {
instance_id: id,
},
saved_objects: [{ type: 'alert', id: params.alertId, namespace: params.namespace }],
saved_objects: [
{
rel: SAVED_OBJECT_REL_PRIMARY,
type: 'alert',
id: params.alertId,
namespace: params.namespace,
},
],
},
message,
};
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/event_log/generated/mappings.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@
},
"saved_objects": {
"properties": {
"rel": {
"type": "keyword",
"ignore_above": 1024
},
"namespace": {
"type": "keyword",
"ignore_above": 1024
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/event_log/generated/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export const EventSchema = schema.maybe(
saved_objects: schema.maybe(
schema.arrayOf(
schema.object({
rel: ecsString(),
namespace: ecsString(),
id: ecsString(),
type: ecsString(),
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/event_log/scripts/mappings.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ exports.EcsKibanaExtensionsMappings = {
saved_objects: {
type: 'nested',
properties: {
// relation; currently only supports "primary" or not set
rel: {
type: 'keyword',
ignore_above: 1024,
},
// relevant kibana space
namespace: {
type: 'keyword',
Expand Down Expand Up @@ -58,6 +63,7 @@ exports.EcsEventLogProperties = [
'user.name',
'kibana.server_uuid',
'kibana.alerting.instance_id',
'kibana.saved_objects.rel',
'kibana.saved_objects.namespace',
'kibana.saved_objects.id',
'kibana.saved_objects.name',
Expand Down
21 changes: 21 additions & 0 deletions x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,13 @@ describe('queryEventsBySavedObject', () => {
query: {
bool: {
must: [
{
term: {
'kibana.saved_objects.rel': {
value: 'primary',
},
},
},
{
term: {
'kibana.saved_objects.type': {
Expand Down Expand Up @@ -319,6 +326,13 @@ describe('queryEventsBySavedObject', () => {
query: {
bool: {
must: [
{
term: {
'kibana.saved_objects.rel': {
value: 'primary',
},
},
},
{
term: {
'kibana.saved_objects.type': {
Expand Down Expand Up @@ -388,6 +402,13 @@ describe('queryEventsBySavedObject', () => {
query: {
bool: {
must: [
{
term: {
'kibana.saved_objects.rel': {
value: 'primary',
},
},
},
{
term: {
'kibana.saved_objects.type': {
Expand Down
9 changes: 8 additions & 1 deletion x-pack/plugins/event_log/server/es/cluster_client_adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { reject, isUndefined } from 'lodash';
import { SearchResponse, Client } from 'elasticsearch';
import { Logger, ClusterClient } from '../../../../../src/core/server';
import { IEvent } from '../types';
import { IEvent, SAVED_OBJECT_REL_PRIMARY } from '../types';
import { FindOptionsType } from '../event_log_client';

export type EsClusterClient = Pick<ClusterClient, 'callAsInternalUser' | 'asScoped'>;
Expand Down Expand Up @@ -155,6 +155,13 @@ export class ClusterClientAdapter {
query: {
bool: {
must: [
{
term: {
'kibana.saved_objects.rel': {
value: SAVED_OBJECT_REL_PRIMARY,
},
},
},
{
term: {
'kibana.saved_objects.type': {
Expand Down
29 changes: 29 additions & 0 deletions x-pack/plugins/event_log/server/event_logger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,35 @@ describe('EventLogger', () => {
message = await waitForLogMessage(systemLogger);
expect(message).toMatch(/invalid event logged.*action.*undefined.*/);
});

test('logs warnings when writing invalid events', async () => {
service.registerProviderActions('provider', ['action-a']);
eventLogger = service.getLogger({});

eventLogger.logEvent(({ event: { PROVIDER: 'provider' } } as unknown) as IEvent);
let message = await waitForLogMessage(systemLogger);
expect(message).toMatch(/invalid event logged.*provider.*undefined.*/);

const event: IEvent = {
event: {
provider: 'provider',
action: 'action-a',
},
kibana: {
saved_objects: [
{
rel: 'ZZZ-primary',
namespace: 'default',
type: 'event_log_test',
id: '123',
},
],
},
};
eventLogger.logEvent(event);
message = await waitForLogMessage(systemLogger);
expect(message).toMatch(/invalid rel property.*ZZZ-primary.*/);
});
});

// return the next logged event; throw if not an event
Expand Down
15 changes: 14 additions & 1 deletion x-pack/plugins/event_log/server/event_logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
ECS_VERSION,
EventSchema,
} from './types';
import { SAVED_OBJECT_REL_PRIMARY } from './types';

type SystemLogger = Plugin['systemLogger'];

Expand Down Expand Up @@ -118,6 +119,8 @@ const RequiredEventSchema = schema.object({
action: schema.string({ minLength: 1 }),
});

const ValidSavedObjectRels = new Set([undefined, SAVED_OBJECT_REL_PRIMARY]);

function validateEvent(eventLogService: IEventLogService, event: IEvent): IValidatedEvent {
if (event?.event == null) {
throw new Error(`no "event" property`);
Expand All @@ -137,7 +140,17 @@ function validateEvent(eventLogService: IEventLogService, event: IEvent): IValid
}

// could throw an error
return EventSchema.validate(event);
const result = EventSchema.validate(event);

if (result?.kibana?.saved_objects?.length) {
for (const so of result?.kibana?.saved_objects) {
if (!ValidSavedObjectRels.has(so.rel)) {
throw new Error(`invalid rel property in saved_objects: "${so.rel}"`);
}
}
}

return result;
}

export const EVENT_LOGGED_PREFIX = `event logged: `;
Expand Down
8 changes: 7 additions & 1 deletion x-pack/plugins/event_log/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ import { PluginInitializerContext } from 'src/core/server';
import { ConfigSchema } from './types';
import { Plugin } from './plugin';

export { IEventLogService, IEventLogger, IEventLogClientService, IEvent } from './types';
export {
IEventLogService,
IEventLogger,
IEventLogClientService,
IEvent,
SAVED_OBJECT_REL_PRIMARY,
} from './types';
export const config = { schema: ConfigSchema };
export const plugin = (context: PluginInitializerContext) => new Plugin(context);
2 changes: 2 additions & 0 deletions x-pack/plugins/event_log/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import { IEvent } from '../generated/schemas';
import { FindOptionsType } from './event_log_client';
import { QueryEventsBySavedObjectResult } from './es/cluster_client_adapter';

export const SAVED_OBJECT_REL_PRIMARY = 'primary';

export const ConfigSchema = schema.object({
enabled: schema.boolean({ defaultValue: true }),
logEntries: schema.boolean({ defaultValue: false }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const logEventRoute = (router: IRouter, eventLogger: IEventLogger, logger
} catch (ex) {
logger.info(`log event error: ${ex}`);
await context.core.savedObjects.client.create('event_log_test', {}, { id });
logger.info(`created saved object`);
logger.info(`created saved object ${id}`);
}
eventLogger.logEvent(event);
logger.info(`logged`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ export default function({ getService }: FtrProviderContext) {
kibana: {
saved_objects: [
{
rel: 'primary',
namespace: 'default',
type: 'event_log_test',
id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export default function({ getService }: FtrProviderContext) {
const eventId = '1';
const event: IEvent = {
event: { action: 'action1', provider: 'provider4' },
kibana: { saved_objects: [{ type: 'event_log_test', id: eventId }] },
kibana: { saved_objects: [{ rel: 'primary', type: 'event_log_test', id: eventId }] },
};
await logTestEvent(eventId, event);

Expand Down

0 comments on commit b8626b1

Please sign in to comment.