[Actions] Added action configuration settings maxResponseContentLength and responseTimeout. (#96355)

* [Actions] Added action configuration settings `maxResponseContentLength` and `responseTimeout` which define max response content size (in bytes) and awaiting timeout for action executions based on axios requests.

* replaced pasceDuration with moment

* fixed due to comments

* renamed internal options
This commit is contained in:
Yuliia Naumenko 2021-04-07 15:06:44 -07:00 committed by GitHub
parent 8d2d2ad864
commit c2d5fa1dda
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 123 additions and 2 deletions

View file

@ -77,6 +77,13 @@ a|`xpack.actions.`
+
As an alternative to setting both `xpack.actions.proxyRejectUnauthorizedCertificates` and `xpack.actions.rejectUnauthorized`, you can point the OS level environment variable `NODE_EXTRA_CA_CERTS` to a file that contains the root CAs needed to trust certificates.
| `xpack.actions.maxResponseContentLength` {ess-icon}
| Specifies the max number of bytes of the http response for requests to external resources. Defaults to 1000000 (1MB).
| `xpack.actions.responseTimeout` {ess-icon}
| Specifies the time allowed for requests to external resources. Requests that take longer are aborted. The time is formatted as <count>[ms|s|m|h|d|w|M|Y], for example, '20m', '24h', '7d', '1w'. Defaults to 60s.
|===
[float]

View file

@ -166,6 +166,8 @@ kibana_vars=(
xpack.actions.proxyBypassHosts
xpack.actions.proxyOnlyHosts
xpack.actions.rejectUnauthorized
xpack.actions.maxResponseContentLength
xpack.actions.responseTimeout
xpack.alerts.healthCheck.interval
xpack.alerts.invalidateApiKeysTask.interval
xpack.alerts.invalidateApiKeysTask.removalDelay

View file

@ -6,6 +6,8 @@
*/
import { schema } from '@kbn/config-schema';
import moment from 'moment';
import { ByteSizeValue } from '@kbn/config-schema';
import { ActionTypeRegistry, ActionTypeRegistryOpts } from './action_type_registry';
import { ActionsClient } from './actions_client';
@ -408,6 +410,8 @@ describe('create()', () => {
rejectUnauthorized: true,
proxyBypassHosts: undefined,
proxyOnlyHosts: undefined,
maxResponseContentLength: new ByteSizeValue(1000000),
responseTimeout: moment.duration('60s'),
});
const localActionTypeRegistryParams = {

View file

@ -17,6 +17,10 @@ const createActionsConfigMock = () => {
ensureActionTypeEnabled: jest.fn().mockReturnValue({}),
isRejectUnauthorizedCertificatesEnabled: jest.fn().mockReturnValue(true),
getProxySettings: jest.fn().mockReturnValue(undefined),
getResponseSettings: jest.fn().mockReturnValue({
maxContentLength: 1000000,
timeout: 360000,
}),
};
return mocked;
};

View file

@ -5,12 +5,14 @@
* 2.0.
*/
import { ByteSizeValue } from '@kbn/config-schema';
import { ActionsConfig } from './config';
import {
getActionsConfigurationUtilities,
AllowedHosts,
EnabledActionTypes,
} from './actions_config';
import moment from 'moment';
const defaultActionsConfig: ActionsConfig = {
enabled: false,
@ -19,6 +21,8 @@ const defaultActionsConfig: ActionsConfig = {
preconfigured: {},
proxyRejectUnauthorizedCertificates: true,
rejectUnauthorized: true,
maxResponseContentLength: new ByteSizeValue(1000000),
responseTimeout: moment.duration(60000),
};
describe('ensureUriAllowed', () => {
@ -254,6 +258,18 @@ describe('ensureActionTypeEnabled', () => {
});
});
describe('getResponseSettingsFromConfig', () => {
test('returns expected parsed values for default config for responseTimeout and maxResponseContentLength', () => {
const config: ActionsConfig = {
...defaultActionsConfig,
};
expect(getActionsConfigurationUtilities(config).getResponseSettings()).toEqual({
timeout: 60000,
maxContentLength: 1000000,
});
});
});
describe('getProxySettings', () => {
test('returns undefined when no proxy URL set', () => {
const config: ActionsConfig = {

View file

@ -13,7 +13,7 @@ import { pipe } from 'fp-ts/lib/pipeable';
import { ActionsConfig, AllowedHosts, EnabledActionTypes } from './config';
import { ActionTypeDisabledError } from './lib';
import { ProxySettings } from './types';
import { ProxySettings, ResponseSettings } from './types';
export { AllowedHosts, EnabledActionTypes } from './config';
@ -31,6 +31,7 @@ export interface ActionsConfigurationUtilities {
ensureActionTypeEnabled: (actionType: string) => void;
isRejectUnauthorizedCertificatesEnabled: () => boolean;
getProxySettings: () => undefined | ProxySettings;
getResponseSettings: () => ResponseSettings;
}
function allowListErrorMessage(field: AllowListingField, value: string) {
@ -99,6 +100,13 @@ function arrayAsSet<T>(arr: T[] | undefined): Set<T> | undefined {
return new Set(arr);
}
function getResponseSettingsFromConfig(config: ActionsConfig): ResponseSettings {
return {
maxContentLength: config.maxResponseContentLength.getValueInBytes(),
timeout: config.responseTimeout.asMilliseconds(),
};
}
export function getActionsConfigurationUtilities(
config: ActionsConfig
): ActionsConfigurationUtilities {
@ -110,6 +118,7 @@ export function getActionsConfigurationUtilities(
isUriAllowed,
isActionTypeEnabled,
getProxySettings: () => getProxySettingsFromConfig(config),
getResponseSettings: () => getResponseSettingsFromConfig(config),
isRejectUnauthorizedCertificatesEnabled: () => config.rejectUnauthorized,
ensureUriAllowed(uri: string) {
if (!isUriAllowed(uri)) {

View file

@ -283,6 +283,7 @@ describe('execute()', () => {
"ensureHostnameAllowed": [MockFunction],
"ensureUriAllowed": [MockFunction],
"getProxySettings": [MockFunction],
"getResponseSettings": [MockFunction],
"isActionTypeEnabled": [MockFunction],
"isHostnameAllowed": [MockFunction],
"isRejectUnauthorizedCertificatesEnabled": [MockFunction],
@ -342,6 +343,7 @@ describe('execute()', () => {
"ensureHostnameAllowed": [MockFunction],
"ensureUriAllowed": [MockFunction],
"getProxySettings": [MockFunction],
"getResponseSettings": [MockFunction],
"isActionTypeEnabled": [MockFunction],
"isHostnameAllowed": [MockFunction],
"isRejectUnauthorizedCertificatesEnabled": [MockFunction],

View file

@ -42,6 +42,10 @@ describe('request', () => {
headers: { 'content-type': 'application/json' },
data: { incidentId: '123' },
}));
configurationUtilities.getResponseSettings.mockReturnValue({
maxContentLength: 1000000,
timeout: 360000,
});
});
test('it fetch correctly with defaults', async () => {
@ -58,6 +62,8 @@ describe('request', () => {
httpAgent: undefined,
httpsAgent: expect.any(HttpsAgent),
proxy: false,
maxContentLength: 1000000,
timeout: 360000,
});
expect(res).toEqual({
status: 200,
@ -88,6 +94,8 @@ describe('request', () => {
httpAgent,
httpsAgent,
proxy: false,
maxContentLength: 1000000,
timeout: 360000,
});
expect(res).toEqual({
status: 200,
@ -116,6 +124,8 @@ describe('request', () => {
httpAgent: undefined,
httpsAgent: expect.any(HttpsAgent),
proxy: false,
maxContentLength: 1000000,
timeout: 360000,
});
expect(res).toEqual({
status: 200,
@ -224,6 +234,8 @@ describe('request', () => {
httpAgent: undefined,
httpsAgent: expect.any(HttpsAgent),
proxy: false,
maxContentLength: 1000000,
timeout: 360000,
});
expect(res).toEqual({
status: 200,
@ -235,10 +247,15 @@ describe('request', () => {
describe('patch', () => {
beforeEach(() => {
jest.resetAllMocks();
axiosMock.mockImplementation(() => ({
status: 200,
headers: { 'content-type': 'application/json' },
}));
configurationUtilities.getResponseSettings.mockReturnValue({
maxContentLength: 1000000,
timeout: 360000,
});
});
test('it fetch correctly', async () => {
@ -249,6 +266,8 @@ describe('patch', () => {
httpAgent: undefined,
httpsAgent: expect.any(HttpsAgent),
proxy: false,
maxContentLength: 1000000,
timeout: 360000,
});
});
});

View file

@ -31,6 +31,7 @@ export const request = async <T = unknown>({
auth?: AxiosBasicCredentials;
}): Promise<AxiosResponse> => {
const { httpAgent, httpsAgent } = getCustomAgents(configurationUtilities, logger, url);
const { maxContentLength, timeout } = configurationUtilities.getResponseSettings();
return await axios(url, {
...rest,
@ -40,6 +41,8 @@ export const request = async <T = unknown>({
httpAgent,
httpsAgent,
proxy: false,
maxContentLength,
timeout,
});
};

View file

@ -168,6 +168,7 @@ describe('execute()', () => {
"ensureHostnameAllowed": [MockFunction],
"ensureUriAllowed": [MockFunction],
"getProxySettings": [MockFunction],
"getResponseSettings": [MockFunction],
"isActionTypeEnabled": [MockFunction],
"isHostnameAllowed": [MockFunction],
"isRejectUnauthorizedCertificatesEnabled": [MockFunction],
@ -230,6 +231,7 @@ describe('execute()', () => {
"ensureHostnameAllowed": [MockFunction],
"ensureUriAllowed": [MockFunction],
"getProxySettings": [MockFunction],
"getResponseSettings": [MockFunction],
"isActionTypeEnabled": [MockFunction],
"isHostnameAllowed": [MockFunction],
"isRejectUnauthorizedCertificatesEnabled": [MockFunction],

View file

@ -291,6 +291,7 @@ describe('execute()', () => {
"ensureHostnameAllowed": [MockFunction],
"ensureUriAllowed": [MockFunction],
"getProxySettings": [MockFunction],
"getResponseSettings": [MockFunction],
"isActionTypeEnabled": [MockFunction],
"isHostnameAllowed": [MockFunction],
"isRejectUnauthorizedCertificatesEnabled": [MockFunction],
@ -329,6 +330,33 @@ describe('execute()', () => {
`);
});
test('execute with exception maxContentLength size exceeded should log the proper error', async () => {
const config: ActionTypeConfigType = {
url: 'https://abc.def/my-webhook',
method: WebhookMethods.POST,
headers: {
aheader: 'a value',
},
hasAuth: true,
};
requestMock.mockReset();
requestMock.mockRejectedValueOnce({
tag: 'err',
isAxiosError: true,
message: 'maxContentLength size of 1000000 exceeded',
});
await actionType.executor({
actionId: 'some-id',
services,
config,
secrets: { user: 'abc', password: '123' },
params: { body: 'some data' },
});
expect(mockedLogger.error).toBeCalledWith(
'error on some-id webhook event: maxContentLength size of 1000000 exceeded'
);
});
test('execute without username/password sends request without basic auth', async () => {
const config: ActionTypeConfigType = {
url: 'https://abc.def/my-webhook',
@ -355,6 +383,7 @@ describe('execute()', () => {
"ensureHostnameAllowed": [MockFunction],
"ensureUriAllowed": [MockFunction],
"getProxySettings": [MockFunction],
"getResponseSettings": [MockFunction],
"isActionTypeEnabled": [MockFunction],
"isHostnameAllowed": [MockFunction],
"isRejectUnauthorizedCertificatesEnabled": [MockFunction],

View file

@ -180,7 +180,6 @@ export async function executor(
return successResult(actionId, data);
} else {
const { error } = result;
if (error.response) {
const {
status,
@ -211,6 +210,10 @@ export async function executor(
const message = `[${error.code}] ${error.message}`;
logger.error(`error on ${actionId} webhook event: ${message}`);
return errorResultRequestFailed(actionId, message);
} else if (error.isAxiosError) {
const message = `${error.message}`;
logger.error(`error on ${actionId} webhook event: ${message}`);
return errorResultRequestFailed(actionId, message);
}
logger.error(`error on ${actionId} webhook action: unexpected error`);

View file

@ -27,9 +27,13 @@ describe('config validation', () => {
"enabledActionTypes": Array [
"*",
],
"maxResponseContentLength": ByteSizeValue {
"valueInBytes": 1048576,
},
"preconfigured": Object {},
"proxyRejectUnauthorizedCertificates": true,
"rejectUnauthorized": true,
"responseTimeout": "PT1M",
}
`);
});
@ -57,6 +61,9 @@ describe('config validation', () => {
"enabledActionTypes": Array [
"*",
],
"maxResponseContentLength": ByteSizeValue {
"valueInBytes": 1048576,
},
"preconfigured": Object {
"mySlack1": Object {
"actionTypeId": ".slack",
@ -69,6 +76,7 @@ describe('config validation', () => {
},
"proxyRejectUnauthorizedCertificates": false,
"rejectUnauthorized": false,
"responseTimeout": "PT1M",
}
`);
});

View file

@ -47,6 +47,8 @@ export const configSchema = schema.object({
proxyBypassHosts: schema.maybe(schema.arrayOf(schema.string({ hostname: true }))),
proxyOnlyHosts: schema.maybe(schema.arrayOf(schema.string({ hostname: true }))),
rejectUnauthorized: schema.boolean({ defaultValue: true }),
maxResponseContentLength: schema.byteSize({ defaultValue: '1mb' }),
responseTimeout: schema.duration({ defaultValue: '60s' }),
});
export type ActionsConfig = TypeOf<typeof configSchema>;

View file

@ -5,6 +5,8 @@
* 2.0.
*/
import moment from 'moment';
import { ByteSizeValue } from '@kbn/config-schema';
import { PluginInitializerContext, RequestHandlerContext } from '../../../../src/core/server';
import { coreMock, httpServerMock } from '../../../../src/core/server/mocks';
import { usageCollectionPluginMock } from '../../../../src/plugins/usage_collection/server/mocks';
@ -37,6 +39,8 @@ describe('Actions Plugin', () => {
preconfigured: {},
proxyRejectUnauthorizedCertificates: true,
rejectUnauthorized: true,
maxResponseContentLength: new ByteSizeValue(1000000),
responseTimeout: moment.duration(60000),
});
plugin = new ActionsPlugin(context);
coreSetup = coreMock.createSetup();
@ -197,6 +201,8 @@ describe('Actions Plugin', () => {
},
proxyRejectUnauthorizedCertificates: true,
rejectUnauthorized: true,
maxResponseContentLength: new ByteSizeValue(1000000),
responseTimeout: moment.duration(60000),
});
plugin = new ActionsPlugin(context);
coreSetup = coreMock.createSetup();

View file

@ -138,3 +138,8 @@ export interface ProxySettings {
proxyHeaders?: Record<string, string>;
proxyRejectUnauthorizedCertificates: boolean;
}
export interface ResponseSettings {
maxContentLength: number;
timeout: number;
}