mirror of
https://github.com/elastic/kibana.git
synced 2025-04-23 01:13:23 -04:00
Skip malformed actions during bulk action scheduling (#192021)
Resolves: #189690 This PR skips only the malformed actions rather than throwing error which blocks execution of the valid actions as well. ### To verify: - Create a pre-configured connectors with below config in the kibana.dev.yml. Notice that slackWebhook doesn't have any `secrets` config. ``` xpack.actions.preconfigured: preconfigured-server-log: name: preconfigured-server-log actionTypeId: .server-log slackWebhook: name: preconfigured-slack-webhook actionTypeId: .slack ``` - Create a rule (e.g. always firing) with 2 actions, one with`preconfigured-server-log` and one with `slackWebhook` - Let the rule run and observe the actions on your terminal. - There should be an error from the slackWebhook whereas the `.server-log` action still works as expected.
This commit is contained in:
Ersin Erdal
GitHub
parent
a141818c4f
commit
bc8fc413c4
3 changed files with 157 additions and 52 deletions
|
|
@ -5,11 +5,11 @@
|
|||
* 2.0.
|
||||
*/
|
||||
|
||||
import { KibanaRequest } from '@kbn/core/server';
|
||||
import { KibanaRequest, Logger } from '@kbn/core/server';
|
||||
import { v4 as uuidv4 } from 'uuid';
|
||||
import { taskManagerMock } from '@kbn/task-manager-plugin/server/mocks';
|
||||
import { createBulkExecutionEnqueuerFunction } from './create_execute_function';
|
||||
import { savedObjectsClientMock } from '@kbn/core/server/mocks';
|
||||
import { loggingSystemMock, savedObjectsClientMock } from '@kbn/core/server/mocks';
|
||||
import { actionTypeRegistryMock } from './action_type_registry.mock';
|
||||
import {
|
||||
asHttpRequestExecutionSource,
|
||||
|
|
@ -21,6 +21,7 @@ const mockTaskManager = taskManagerMock.createStart();
|
|||
const savedObjectsClient = savedObjectsClientMock.create();
|
||||
const request = {} as KibanaRequest;
|
||||
const mockActionsConfig = actionsConfigMock.create();
|
||||
const mockLogger = loggingSystemMock.create().get() as jest.Mocked<Logger>;
|
||||
|
||||
beforeEach(() => {
|
||||
jest.resetAllMocks();
|
||||
|
|
@ -43,6 +44,7 @@ describe('bulkExecute()', () => {
|
|||
isESOCanEncrypt: true,
|
||||
inMemoryConnectors: [],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
savedObjectsClient.bulkGet.mockResolvedValueOnce({
|
||||
saved_objects: [
|
||||
|
|
@ -133,6 +135,7 @@ describe('bulkExecute()', () => {
|
|||
isESOCanEncrypt: true,
|
||||
inMemoryConnectors: [],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
savedObjectsClient.bulkGet.mockResolvedValueOnce({
|
||||
saved_objects: [
|
||||
|
|
@ -226,6 +229,7 @@ describe('bulkExecute()', () => {
|
|||
isESOCanEncrypt: true,
|
||||
inMemoryConnectors: [],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
savedObjectsClient.bulkGet.mockResolvedValueOnce({
|
||||
saved_objects: [
|
||||
|
|
@ -323,6 +327,7 @@ describe('bulkExecute()', () => {
|
|||
},
|
||||
],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
const source = { type: 'alert', id: uuidv4() };
|
||||
|
||||
|
|
@ -422,6 +427,7 @@ describe('bulkExecute()', () => {
|
|||
},
|
||||
],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
const source = { type: 'alert', id: uuidv4() };
|
||||
|
||||
|
|
@ -521,6 +527,7 @@ describe('bulkExecute()', () => {
|
|||
},
|
||||
],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
const source = { type: 'alert', id: uuidv4() };
|
||||
|
||||
|
|
@ -641,6 +648,7 @@ describe('bulkExecute()', () => {
|
|||
},
|
||||
],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
const source = { type: 'alert', id: uuidv4() };
|
||||
|
||||
|
|
@ -750,6 +758,7 @@ describe('bulkExecute()', () => {
|
|||
actionTypeRegistry: actionTypeRegistryMock.create(),
|
||||
inMemoryConnectors: [],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
await expect(
|
||||
executeFn(savedObjectsClient, [
|
||||
|
|
@ -768,13 +777,14 @@ describe('bulkExecute()', () => {
|
|||
);
|
||||
});
|
||||
|
||||
test('throws when isMissingSecrets is true for connector', async () => {
|
||||
test('filter outs the connectors when isMissingSecrets is true', async () => {
|
||||
const executeFn = createBulkExecutionEnqueuerFunction({
|
||||
taskManager: mockTaskManager,
|
||||
isESOCanEncrypt: true,
|
||||
actionTypeRegistry: actionTypeRegistryMock.create(),
|
||||
inMemoryConnectors: [],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
savedObjectsClient.bulkGet.mockResolvedValueOnce({
|
||||
saved_objects: [
|
||||
|
|
@ -788,26 +798,69 @@ describe('bulkExecute()', () => {
|
|||
},
|
||||
references: [],
|
||||
},
|
||||
{
|
||||
id: '234',
|
||||
type: 'action',
|
||||
attributes: {
|
||||
name: 'mock-action-2',
|
||||
isMissingSecrets: false,
|
||||
actionTypeId: 'mock-action-2',
|
||||
},
|
||||
references: [],
|
||||
},
|
||||
],
|
||||
});
|
||||
await expect(
|
||||
executeFn(savedObjectsClient, [
|
||||
savedObjectsClient.bulkCreate.mockResolvedValueOnce({
|
||||
saved_objects: [
|
||||
{
|
||||
id: '123',
|
||||
params: { baz: false },
|
||||
spaceId: 'default',
|
||||
executionId: '123abc',
|
||||
apiKey: null,
|
||||
source: asHttpRequestExecutionSource(request),
|
||||
actionTypeId: 'mock-action',
|
||||
id: '234',
|
||||
type: 'action_task_params',
|
||||
attributes: {
|
||||
actionId: '123',
|
||||
},
|
||||
references: [],
|
||||
},
|
||||
])
|
||||
).rejects.toThrowErrorMatchingInlineSnapshot(
|
||||
`"Unable to execute action because no secrets are defined for the \\"mock-action\\" connector."`
|
||||
],
|
||||
});
|
||||
|
||||
const result = await executeFn(savedObjectsClient, [
|
||||
{
|
||||
id: '123',
|
||||
params: { baz: false },
|
||||
spaceId: 'default',
|
||||
executionId: '123abc',
|
||||
apiKey: null,
|
||||
source: asHttpRequestExecutionSource(request),
|
||||
actionTypeId: 'mock-action',
|
||||
},
|
||||
{
|
||||
id: '234',
|
||||
params: { baz: false },
|
||||
spaceId: 'default',
|
||||
executionId: '123abc',
|
||||
apiKey: null,
|
||||
source: asHttpRequestExecutionSource(request),
|
||||
actionTypeId: 'mock-action-2',
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result).toEqual({
|
||||
errors: false,
|
||||
items: [
|
||||
{
|
||||
actionTypeId: 'mock-action-2',
|
||||
id: '234',
|
||||
response: 'success',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(mockLogger.warn).toHaveBeenCalledWith(
|
||||
'Skipped the actions for the connector: mock-action (123). Error: Unable to execute action because no secrets are defined for the "mock-action" connector.'
|
||||
);
|
||||
});
|
||||
|
||||
test('should ensure action type is enabled', async () => {
|
||||
test('skips the disabled action types', async () => {
|
||||
const mockedActionTypeRegistry = actionTypeRegistryMock.create();
|
||||
const executeFn = createBulkExecutionEnqueuerFunction({
|
||||
taskManager: mockTaskManager,
|
||||
|
|
@ -815,6 +868,7 @@ describe('bulkExecute()', () => {
|
|||
actionTypeRegistry: mockedActionTypeRegistry,
|
||||
inMemoryConnectors: [],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
mockedActionTypeRegistry.ensureActionTypeEnabled.mockImplementation(() => {
|
||||
throw new Error('Fail');
|
||||
|
|
@ -826,25 +880,45 @@ describe('bulkExecute()', () => {
|
|||
type: 'action',
|
||||
attributes: {
|
||||
actionTypeId: 'mock-action',
|
||||
name: 'test-connector',
|
||||
},
|
||||
references: [],
|
||||
},
|
||||
],
|
||||
});
|
||||
savedObjectsClient.bulkCreate.mockResolvedValueOnce({
|
||||
saved_objects: [
|
||||
{
|
||||
id: '234',
|
||||
type: 'action_task_params',
|
||||
attributes: {
|
||||
actionId: '123',
|
||||
},
|
||||
references: [],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await expect(
|
||||
executeFn(savedObjectsClient, [
|
||||
{
|
||||
id: '123',
|
||||
params: { baz: false },
|
||||
spaceId: 'default',
|
||||
executionId: '123abc',
|
||||
apiKey: null,
|
||||
source: asHttpRequestExecutionSource(request),
|
||||
actionTypeId: 'mock-action',
|
||||
},
|
||||
])
|
||||
).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`);
|
||||
const result = await executeFn(savedObjectsClient, [
|
||||
{
|
||||
id: '123',
|
||||
params: { baz: false },
|
||||
spaceId: 'default',
|
||||
executionId: '123abc',
|
||||
apiKey: null,
|
||||
source: asHttpRequestExecutionSource(request),
|
||||
actionTypeId: 'mock-action',
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result).toEqual({
|
||||
errors: false,
|
||||
items: [],
|
||||
});
|
||||
|
||||
expect(mockLogger.warn).toHaveBeenCalledWith(
|
||||
'Skipped the actions for the connector: test-connector (123). Error: Fail'
|
||||
);
|
||||
});
|
||||
|
||||
test('should skip ensure action type if action type is preconfigured and license is valid', async () => {
|
||||
|
|
@ -866,6 +940,7 @@ describe('bulkExecute()', () => {
|
|||
},
|
||||
],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
mockedActionTypeRegistry.isActionExecutable.mockImplementation(() => true);
|
||||
savedObjectsClient.bulkGet.mockResolvedValueOnce({
|
||||
|
|
@ -927,6 +1002,7 @@ describe('bulkExecute()', () => {
|
|||
},
|
||||
],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
mockedActionTypeRegistry.isActionExecutable.mockImplementation(() => true);
|
||||
savedObjectsClient.bulkGet.mockResolvedValueOnce({
|
||||
|
|
@ -984,6 +1060,7 @@ describe('bulkExecute()', () => {
|
|||
isESOCanEncrypt: true,
|
||||
inMemoryConnectors: [],
|
||||
configurationUtilities: mockActionsConfig,
|
||||
logger: mockLogger,
|
||||
});
|
||||
savedObjectsClient.bulkGet.mockResolvedValueOnce({
|
||||
saved_objects: [],
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@
|
|||
* 2.0.
|
||||
*/
|
||||
|
||||
import { SavedObjectsBulkResponse, SavedObjectsClientContract } from '@kbn/core/server';
|
||||
import { SavedObjectsBulkResponse, SavedObjectsClientContract, Logger } from '@kbn/core/server';
|
||||
import { RunNowResult, TaskManagerStartContract } from '@kbn/task-manager-plugin/server';
|
||||
import {
|
||||
RawAction,
|
||||
|
|
@ -25,6 +25,7 @@ interface CreateExecuteFunctionOptions {
|
|||
actionTypeRegistry: ActionTypeRegistryContract;
|
||||
inMemoryConnectors: InMemoryConnector[];
|
||||
configurationUtilities: ActionsConfigurationUtilities;
|
||||
logger: Logger;
|
||||
}
|
||||
|
||||
export interface ExecuteOptions
|
||||
|
|
@ -80,6 +81,7 @@ export function createBulkExecutionEnqueuerFunction({
|
|||
isESOCanEncrypt,
|
||||
inMemoryConnectors,
|
||||
configurationUtilities,
|
||||
logger,
|
||||
}: CreateExecuteFunctionOptions): BulkExecutionEnqueuer<ExecutionResponse> {
|
||||
return async function execute(
|
||||
unsecuredSavedObjectsClient: SavedObjectsClientContract,
|
||||
|
|
@ -113,21 +115,30 @@ export function createBulkExecutionEnqueuerFunction({
|
|||
inMemoryConnectors,
|
||||
connectorIds
|
||||
);
|
||||
const runnableActions: ExecuteOptions[] = [];
|
||||
|
||||
connectors.forEach((c) => {
|
||||
for (const c of connectors) {
|
||||
const { id, connector, isInMemory } = c;
|
||||
validateCanActionBeUsed(connector);
|
||||
const { actionTypeId, name } = connector;
|
||||
|
||||
const { actionTypeId } = connector;
|
||||
if (!actionTypeRegistry.isActionExecutable(id, actionTypeId, { notifyUsage: true })) {
|
||||
actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);
|
||||
try {
|
||||
validateConnector({ id, connector, actionTypeRegistry });
|
||||
} catch (e) {
|
||||
logger.warn(`Skipped the actions for the connector: ${name} (${id}). Error: ${e.message}`);
|
||||
continue;
|
||||
}
|
||||
|
||||
const actionsWithCurrentConnector = actionsToExecute.filter((action) => action.id === id);
|
||||
|
||||
if (actionsWithCurrentConnector.length > 0) {
|
||||
runnableActions.push(...actionsWithCurrentConnector);
|
||||
}
|
||||
|
||||
actionTypeIds[id] = actionTypeId;
|
||||
connectorIsInMemory[id] = isInMemory;
|
||||
});
|
||||
}
|
||||
|
||||
const actions = actionsToExecute.map((actionToExecute) => {
|
||||
const actions = runnableActions.map((actionToExecute) => {
|
||||
// Get saved object references from action ID and relatedSavedObjects
|
||||
const { references, relatedSavedObjectWithRefs } = extractSavedObjectReferences(
|
||||
actionToExecute.id,
|
||||
|
|
@ -165,6 +176,7 @@ export function createBulkExecutionEnqueuerFunction({
|
|||
});
|
||||
const actionTaskParamsRecords: SavedObjectsBulkResponse<ActionTaskParams> =
|
||||
await unsecuredSavedObjectsClient.bulkCreate(actions, { refresh: false });
|
||||
|
||||
const taskInstances = actionTaskParamsRecords.saved_objects.map((so) => {
|
||||
const actionId = so.attributes.actionId;
|
||||
return {
|
||||
|
|
@ -177,10 +189,12 @@ export function createBulkExecutionEnqueuerFunction({
|
|||
scope: ['actions'],
|
||||
};
|
||||
});
|
||||
|
||||
await taskManager.bulkSchedule(taskInstances);
|
||||
|
||||
return {
|
||||
errors: actionsOverLimit.length > 0,
|
||||
items: actionsToExecute
|
||||
items: runnableActions
|
||||
.map((a) => ({
|
||||
id: a.id,
|
||||
actionTypeId: a.actionTypeId,
|
||||
|
|
@ -201,18 +215,15 @@ export function createEphemeralExecutionEnqueuerFunction({
|
|||
taskManager,
|
||||
actionTypeRegistry,
|
||||
inMemoryConnectors,
|
||||
logger,
|
||||
}: CreateExecuteFunctionOptions): ExecutionEnqueuer<RunNowResult> {
|
||||
return async function execute(
|
||||
unsecuredSavedObjectsClient: SavedObjectsClientContract,
|
||||
{ id, params, spaceId, source, consumer, apiKey, executionId }: ExecuteOptions
|
||||
): Promise<RunNowResult> {
|
||||
const { action } = await getAction(unsecuredSavedObjectsClient, inMemoryConnectors, id);
|
||||
validateCanActionBeUsed(action);
|
||||
const { connector } = await getConnector(unsecuredSavedObjectsClient, inMemoryConnectors, id);
|
||||
|
||||
const { actionTypeId } = action;
|
||||
if (!actionTypeRegistry.isActionExecutable(id, actionTypeId, { notifyUsage: true })) {
|
||||
actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);
|
||||
}
|
||||
validateConnector({ id, connector, actionTypeRegistry });
|
||||
|
||||
const taskParams: ActionTaskExecutorParams = {
|
||||
spaceId,
|
||||
|
|
@ -229,7 +240,7 @@ export function createEphemeralExecutionEnqueuerFunction({
|
|||
};
|
||||
|
||||
return taskManager.ephemeralRunNow({
|
||||
taskType: `actions:${action.actionTypeId}`,
|
||||
taskType: `actions:${connector.actionTypeId}`,
|
||||
params: taskParams,
|
||||
state: {},
|
||||
scope: ['actions'],
|
||||
|
|
@ -237,13 +248,26 @@ export function createEphemeralExecutionEnqueuerFunction({
|
|||
};
|
||||
}
|
||||
|
||||
function validateCanActionBeUsed(action: InMemoryConnector | RawAction) {
|
||||
const { name, isMissingSecrets } = action;
|
||||
function validateConnector({
|
||||
id,
|
||||
connector,
|
||||
actionTypeRegistry,
|
||||
}: {
|
||||
id: string;
|
||||
connector: InMemoryConnector | RawAction;
|
||||
actionTypeRegistry: ActionTypeRegistryContract;
|
||||
}) {
|
||||
const { name, isMissingSecrets, actionTypeId } = connector;
|
||||
|
||||
if (isMissingSecrets) {
|
||||
throw new Error(
|
||||
`Unable to execute action because no secrets are defined for the "${name}" connector.`
|
||||
);
|
||||
}
|
||||
|
||||
if (!actionTypeRegistry.isActionExecutable(id, actionTypeId, { notifyUsage: true })) {
|
||||
actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);
|
||||
}
|
||||
}
|
||||
|
||||
function executionSourceAsSavedObjectReferences(executionSource: ActionExecutorOptions['source']) {
|
||||
|
|
@ -259,19 +283,19 @@ function executionSourceAsSavedObjectReferences(executionSource: ActionExecutorO
|
|||
: {};
|
||||
}
|
||||
|
||||
async function getAction(
|
||||
async function getConnector(
|
||||
unsecuredSavedObjectsClient: SavedObjectsClientContract,
|
||||
inMemoryConnectors: InMemoryConnector[],
|
||||
actionId: string
|
||||
): Promise<{ action: InMemoryConnector | RawAction; isInMemory: boolean }> {
|
||||
): Promise<{ connector: InMemoryConnector | RawAction; isInMemory: boolean }> {
|
||||
const inMemoryAction = inMemoryConnectors.find((action) => action.id === actionId);
|
||||
|
||||
if (inMemoryAction) {
|
||||
return { action: inMemoryAction, isInMemory: true };
|
||||
return { connector: inMemoryAction, isInMemory: true };
|
||||
}
|
||||
|
||||
const { attributes } = await unsecuredSavedObjectsClient.get<RawAction>('action', actionId);
|
||||
return { action: attributes, isInMemory: false };
|
||||
return { connector: attributes, isInMemory: false };
|
||||
}
|
||||
|
||||
async function getConnectors(
|
||||
|
|
|
|||
|
|
@ -478,6 +478,7 @@ export class ActionsPlugin implements Plugin<PluginSetupContract, PluginStartCon
|
|||
isESOCanEncrypt: isESOCanEncrypt!,
|
||||
inMemoryConnectors: this.inMemoryConnectors,
|
||||
configurationUtilities: actionsConfigUtils,
|
||||
logger,
|
||||
}),
|
||||
bulkExecutionEnqueuer: createBulkExecutionEnqueuerFunction({
|
||||
taskManager: plugins.taskManager,
|
||||
|
|
@ -485,13 +486,14 @@ export class ActionsPlugin implements Plugin<PluginSetupContract, PluginStartCon
|
|||
isESOCanEncrypt: isESOCanEncrypt!,
|
||||
inMemoryConnectors: this.inMemoryConnectors,
|
||||
configurationUtilities: actionsConfigUtils,
|
||||
logger,
|
||||
}),
|
||||
auditLogger: this.security?.audit.asScoped(request),
|
||||
usageCounter: this.usageCounter,
|
||||
connectorTokenClient: new ConnectorTokenClient({
|
||||
unsecuredSavedObjectsClient,
|
||||
encryptedSavedObjectsClient,
|
||||
logger: this.logger,
|
||||
logger,
|
||||
}),
|
||||
async getEventLogClient() {
|
||||
return plugins.eventLog.getClient(request);
|
||||
|
|
@ -754,6 +756,7 @@ export class ActionsPlugin implements Plugin<PluginSetupContract, PluginStartCon
|
|||
isESOCanEncrypt: isESOCanEncrypt!,
|
||||
inMemoryConnectors,
|
||||
configurationUtilities: actionsConfigUtils,
|
||||
logger,
|
||||
}),
|
||||
bulkExecutionEnqueuer: createBulkExecutionEnqueuerFunction({
|
||||
taskManager,
|
||||
|
|
@ -761,6 +764,7 @@ export class ActionsPlugin implements Plugin<PluginSetupContract, PluginStartCon
|
|||
isESOCanEncrypt: isESOCanEncrypt!,
|
||||
inMemoryConnectors,
|
||||
configurationUtilities: actionsConfigUtils,
|
||||
logger,
|
||||
}),
|
||||
auditLogger: security?.audit.asScoped(request),
|
||||
usageCounter,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue