maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[SIEM][Detection Engine][Lists] Adds the ability to change the timeout limits from 10 seconds for loads for imports (#73103)

Browse source 
## Summary

By default the upload time limit for payloads is 10 seconds. This is really too short and we were getting internal QA bug reports that uploads are timing out on large value list importing. This PR adds the plumbing and unit tests to make the timeout configurable for routes.

* Adds a single timeout option for routes and then normalizes that through Hapi for the socket, payload, and server timeouts.
* Adds unit tests which test the various options
* Adds integration tests which test the various options
* Adds some NOTES about where there are odd behaviors/bugs within Hapi around validations and the timeouts
* Adds a configurable 5 minute timeout to the large value lists route

**Manual testing of the feature**

You can manually test this by adding a configurable option to your chrome network throttle like so below where you throttle upload by some configurable amount. I chose to use 300 kbs/s upload
<img width="556" alt="Screen Shot 2020-07-23 at 11 26 01 AM" src="https://user-images.githubusercontent.com/1151048/88318015-5ab3f700-ccd7-11ea-9d9b-7e3649ec65de.png">

And then run an import of large value lists using a large enough file that it will exceed 5 minutes:
![screen-shot-upload](https://user-images.githubusercontent.com/1151048/88318584-28ef6000-ccd8-11ea-90a1-8ca4aafabcb4.png)

After 5 minutes you should see this message within your server side messages if you have configured your kibana.dev.yml to allow for these messages:

```ts
server  respons [10:52:31.377] [access:lists-all] POST /api/lists/items/_import?type=keyword 408 318292ms - 9.0B
``` 

Note that it should show you that it is trying to return a `408` after `318292ms` the timeout period. Sometimes you will get the 408 in the browser and sometimes the browser actually will not respect the 408 and continue staying in a pending state forever. This seems to be browser side issue and not a client/user land issue. If you get the browser message it will be this error toaster

![timeout-message](https://user-images.githubusercontent.com/1151048/88318760-74a20980-ccd8-11ea-9b7b-0d27f8eb6bce.png)

### Checklist

- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [x] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials
- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios

### For maintainers

- [x] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
This commit is contained in:
Frank Hassanabad 2020-07-28 17:47:41 -06:00 committed by GitHub
parent b65ec4e07d
commit b399fb03d1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 333 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
docs/development/core/server/kibana-plugin-core-server.routeconfigoptions.md
View file

@ -19,5 +19,6 @@ export interface RouteConfigOptions<Method extends RouteMethod>
| [authRequired](./kibana-plugin-core-server.routeconfigoptions.authrequired.md) | <code>boolean &#124; 'optional'</code> | Defines authentication mode for a route: - true. A user has to have valid credentials to access a resource - false. A user can access a resource without any credentials. - 'optional'. A user can access a resource if has valid credentials or no credentials at all. Can be useful when we grant access to a resource but want to identify a user if possible.<!-- -->Defaults to <code>true</code> if an auth mechanism is registered. |
| [body](./kibana-plugin-core-server.routeconfigoptions.body.md) | <code>Method extends 'get' &#124; 'options' ? undefined : RouteConfigOptionsBody</code> | Additional body options [RouteConfigOptionsBody](./kibana-plugin-core-server.routeconfigoptionsbody.md)<!-- -->. |
| [tags](./kibana-plugin-core-server.routeconfigoptions.tags.md) | <code>readonly string[]</code> | Additional metadata tag strings to attach to the route. |
| [timeout](./kibana-plugin-core-server.routeconfigoptions.timeout.md) | <code>number</code> | Timeouts for processing durations. Response timeout is in milliseconds. Default value: 2 minutes |
| [xsrfRequired](./kibana-plugin-core-server.routeconfigoptions.xsrfrequired.md) | <code>Method extends 'get' ? never : boolean</code> | Defines xsrf protection requirements for a route: - true. Requires an incoming POST/PUT/DELETE request to contain <code>kbn-xsrf</code> header. - false. Disables xsrf protection.<!-- -->Set to true by default |

13
docs/development/core/server/kibana-plugin-core-server.routeconfigoptions.timeout.md Normal file
View file

@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [RouteConfigOptions](./kibana-plugin-core-server.routeconfigoptions.md) &gt; [timeout](./kibana-plugin-core-server.routeconfigoptions.timeout.md)
## RouteConfigOptions.timeout property
Timeouts for processing durations. Response timeout is in milliseconds. Default value: 2 minutes
<b>Signature:</b>
```typescript
timeout?: number;
```

127
src/core/server/http/http_server.test.ts
View file

@ -992,6 +992,133 @@ describe('body options', () => {
});
});
describe('timeout options', () => {
test('should accept a socket "timeout" which is 3 minutes in milliseconds, "300000" for a POST', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
const router = new Router('', logger, enhanceWithContext);
router.post(
{
path: '/',
validate: false,
options: { timeout: 300000 },
},
(context, req, res) => {
try {
return res.ok({ body: { timeout: req.route.options.timeout } });
} catch (err) {
return res.internalError({ body: err.message });
}
}
);
registerRouter(router);
await server.start();
await supertest(innerServer.listener).post('/').send({ test: 1 }).expect(200, {
timeout: 300000,
});
});
test('should accept a socket "timeout" which is 3 minutes in milliseconds, "300000" for a GET', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
const router = new Router('', logger, enhanceWithContext);
router.get(
{
path: '/',
validate: false,
options: { timeout: 300000 },
},
(context, req, res) => {
try {
return res.ok({ body: { timeout: req.route.options.timeout } });
} catch (err) {
return res.internalError({ body: err.message });
}
}
);
registerRouter(router);
await server.start();
await supertest(innerServer.listener).get('/').expect(200, {
timeout: 300000,
});
});
test('should accept a socket "timeout" which is 3 minutes in milliseconds, "300000" for a DELETE', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
const router = new Router('', logger, enhanceWithContext);
router.delete(
{
path: '/',
validate: false,
options: { timeout: 300000 },
},
(context, req, res) => {
try {
return res.ok({ body: { timeout: req.route.options.timeout } });
} catch (err) {
return res.internalError({ body: err.message });
}
}
);
registerRouter(router);
await server.start();
await supertest(innerServer.listener).delete('/').expect(200, {
timeout: 300000,
});
});
test('should accept a socket "timeout" which is 3 minutes in milliseconds, "300000" for a PUT', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
const router = new Router('', logger, enhanceWithContext);
router.put(
{
path: '/',
validate: false,
options: { timeout: 300000 },
},
(context, req, res) => {
try {
return res.ok({ body: { timeout: req.route.options.timeout } });
} catch (err) {
return res.internalError({ body: err.message });
}
}
);
registerRouter(router);
await server.start();
await supertest(innerServer.listener).put('/').expect(200, {
timeout: 300000,
});
});
test('should accept a socket "timeout" which is 3 minutes in milliseconds, "300000" for a PATCH', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
const router = new Router('', logger, enhanceWithContext);
router.patch(
{
path: '/',
validate: false,
options: { timeout: 300000 },
},
(context, req, res) => {
try {
return res.ok({ body: { timeout: req.route.options.timeout } });
} catch (err) {
return res.internalError({ body: err.message });
}
}
);
registerRouter(router);
await server.start();
await supertest(innerServer.listener).patch('/').expect(200, {
timeout: 300000,
});
});
});
test('should return a stream in the body', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);

22
src/core/server/http/http_server.ts
View file

@ -161,8 +161,10 @@ export class HttpServer {
this.log.debug(`registering route handler for [${route.path}]`);
// Hapi does not allow payload validation to be specified for 'head' or 'get' requests
const validate = isSafeMethod(route.method) ? undefined : { payload: true };
const { authRequired, tags, body = {} } = route.options;
const { authRequired, tags, body = {}, timeout } = route.options;
const { accepts: allow, maxBytes, output, parse } = body;
// Hapi does not allow timeouts on payloads to be specified for 'head' or 'get' requests
const payloadTimeout = isSafeMethod(route.method) || timeout == null ? undefined : timeout;
const kibanaRouteState: KibanaRouteState = {
xsrfRequired: route.options.xsrfRequired ?? !isSafeMethod(route.method),
@ -181,9 +183,23 @@ export class HttpServer {
// validation applied in ./http_tools#getServerOptions
// (All NP routes are already required to specify their own validation in order to access the payload)
validate,
payload: [allow, maxBytes, output, parse].some((v) => typeof v !== 'undefined')
? { allow, maxBytes, output, parse }
payload: [allow, maxBytes, output, parse, payloadTimeout].some(
(v) => typeof v !== 'undefined'
)
? {
allow,
maxBytes,
output,
parse,
timeout: payloadTimeout,
}
: undefined,
timeout:
timeout != null
? {
socket: timeout + 1, // Hapi server requires the socket to be greater than payload settings so we add 1 millisecond
}
: undefined,
},
});
}

124
src/core/server/http/integration_tests/router.test.ts
View file

@ -302,6 +302,130 @@ describe('Options', () => {
});
});
});
describe('timeout', () => {
it('should timeout if configured with a small timeout value for a POST', async () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
const router = createRouter('/');
router.post(
{ path: '/a', validate: false, options: { timeout: 1000 } },
async (context, req, res) => {
await new Promise((resolve) => setTimeout(resolve, 2000));
return res.ok({});
}
);
router.post({ path: '/b', validate: false }, (context, req, res) => res.ok({}));
await server.start();
expect(supertest(innerServer.listener).post('/a')).rejects.toThrow('socket hang up');
await supertest(innerServer.listener).post('/b').expect(200, {});
});
it('should timeout if configured with a small timeout value for a PUT', async () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
const router = createRouter('/');
router.put(
{ path: '/a', validate: false, options: { timeout: 1000 } },
async (context, req, res) => {
await new Promise((resolve) => setTimeout(resolve, 2000));
return res.ok({});
}
);
router.put({ path: '/b', validate: false }, (context, req, res) => res.ok({}));
await server.start();
expect(supertest(innerServer.listener).put('/a')).rejects.toThrow('socket hang up');
await supertest(innerServer.listener).put('/b').expect(200, {});
});
it('should timeout if configured with a small timeout value for a DELETE', async () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
const router = createRouter('/');
router.delete(
{ path: '/a', validate: false, options: { timeout: 1000 } },
async (context, req, res) => {
await new Promise((resolve) => setTimeout(resolve, 2000));
return res.ok({});
}
);
router.delete({ path: '/b', validate: false }, (context, req, res) => res.ok({}));
await server.start();
expect(supertest(innerServer.listener).delete('/a')).rejects.toThrow('socket hang up');
await supertest(innerServer.listener).delete('/b').expect(200, {});
});
it('should timeout if configured with a small timeout value for a GET', async () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
const router = createRouter('/');
router.get(
// Note: There is a bug within Hapi Server where it cannot set the payload timeout for a GET call but it also cannot configure a timeout less than the payload body
// so the least amount of possible time to configure the timeout is 10 seconds.
{ path: '/a', validate: false, options: { timeout: 100000 } },
async (context, req, res) => {
// Cause a wait of 20 seconds to cause the socket hangup
await new Promise((resolve) => setTimeout(resolve, 200000));
return res.ok({});
}
);
router.get({ path: '/b', validate: false }, (context, req, res) => res.ok({}));
await server.start();
expect(supertest(innerServer.listener).get('/a')).rejects.toThrow('socket hang up');
await supertest(innerServer.listener).get('/b').expect(200, {});
});
it('should not timeout if configured with a 5 minute timeout value for a POST', async () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
const router = createRouter('/');
router.post(
{ path: '/a', validate: false, options: { timeout: 300000 } },
async (context, req, res) => res.ok({})
);
await server.start();
await supertest(innerServer.listener).post('/a').expect(200, {});
});
it('should not timeout if configured with a 5 minute timeout value for a PUT', async () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
const router = createRouter('/');
router.put(
{ path: '/a', validate: false, options: { timeout: 300000 } },
async (context, req, res) => res.ok({})
);
await server.start();
await supertest(innerServer.listener).put('/a').expect(200, {});
});
it('should not timeout if configured with a 5 minute timeout value for a DELETE', async () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
const router = createRouter('/');
router.delete(
{ path: '/a', validate: false, options: { timeout: 300000 } },
async (context, req, res) => res.ok({})
);
await server.start();
await supertest(innerServer.listener).delete('/a').expect(200, {});
});
it('should not timeout if configured with a 5 minute timeout value for a GET', async () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
const router = createRouter('/');
router.get(
{ path: '/a', validate: false, options: { timeout: 300000 } },
async (context, req, res) => res.ok({})
);
await server.start();
await supertest(innerServer.listener).get('/a').expect(200, {});
});
});
});
describe('Cache-Control', () => {

2
src/core/server/http/router/request.ts
View file

@ -197,12 +197,14 @@ export class KibanaRequest<
private getRouteInfo(request: Request): KibanaRequestRoute<Method> {
const method = request.method as Method;
const { parse, maxBytes, allow, output } = request.route.settings.payload || {};
const timeout = request.route.settings.timeout?.socket;
const options = ({
authRequired: this.getAuthRequired(request),
// some places in LP call KibanaRequest.from(request) manually. remove fallback to true before v8
xsrfRequired: (request.route.settings.app as KibanaRouteState)?.xsrfRequired ?? true,
tags: request.route.settings.tags || [],
timeout: typeof timeout === 'number' ? timeout - 1 : undefined, // We are forced to have the timeout be 1 millisecond greater than the server and payload so we subtract one here to give the user consist settings
body: isSafeMethod(method)
? undefined
: {

6
src/core/server/http/router/route.ts
View file

@ -144,6 +144,12 @@ export interface RouteConfigOptions<Method extends RouteMethod> {
* Additional body options {@link RouteConfigOptionsBody}.
*/
body?: Method extends 'get' | 'options' ? undefined : RouteConfigOptionsBody;
/**
* Timeouts for processing durations. Response timeout is in milliseconds.
* Default value: 2 minutes
*/
timeout?: number;
}
/**

1
src/core/server/server.api.md
View file

@ -1859,6 +1859,7 @@ export interface RouteConfigOptions<Method extends RouteMethod> {
authRequired?: boolean | 'optional';
body?: Method extends 'get' | 'options' ? undefined : RouteConfigOptionsBody;
tags?: readonly string[];
timeout?: number;
xsrfRequired?: Method extends 'get' ? never : boolean;
}

4
x-pack/plugins/lists/common/constants.mock.ts
View file

@ -3,8 +3,9 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { EntriesArray } from './schemas/types';
import moment from 'moment';
import { EntriesArray } from './schemas/types';
export const DATE_NOW = '2020-04-20T15:25:31.830Z';
export const OLD_DATE_RELATIVE_TO_DATE_NOW = '2020-04-19T15:25:31.830Z';
export const USER = 'some user';
@ -64,3 +65,4 @@ export const CURSOR = 'c29tZXN0cmluZ2ZvcnlvdQ==';
export const _VERSION = 'WzI5NywxXQ==';
export const VERSION = 1;
export const IMMUTABLE = false;
export const IMPORT_TIMEOUT = moment.duration(5, 'minutes');

2
x-pack/plugins/lists/server/config.mock.ts
View file

@ -6,6 +6,7 @@
import {
IMPORT_BUFFER_SIZE,
IMPORT_TIMEOUT,
LIST_INDEX,
LIST_ITEM_INDEX,
MAX_IMPORT_PAYLOAD_BYTES,
@ -21,6 +22,7 @@ export const getConfigMock = (): Partial<ConfigType> => ({
export const getConfigMockDecoded = (): ConfigType => ({
enabled: true,
importBufferSize: IMPORT_BUFFER_SIZE,
importTimeout: IMPORT_TIMEOUT,
listIndex: LIST_INDEX,
listItemIndex: LIST_ITEM_INDEX,
maxImportPayloadBytes: MAX_IMPORT_PAYLOAD_BYTES,

22
x-pack/plugins/lists/server/config.test.ts
View file

@ -4,6 +4,8 @@
* you may not use this file except in compliance with the Elastic License.
*/
import moment from 'moment';
import { ConfigSchema, ConfigType } from './config';
import { getConfigMock, getConfigMockDecoded } from './config.mock';
@ -61,4 +63,24 @@ describe('config_schema', () => {
'[importBufferSize]: Value must be equal to or greater than [1].'
);
});
test('it throws if the "importTimeout" value is less than 2 minutes', () => {
const mock: ConfigType = {
...getConfigMockDecoded(),
importTimeout: moment.duration(2, 'minutes').subtract(1, 'second'),
};
expect(() => ConfigSchema.validate(mock)).toThrow(
'[importTimeout]: duration cannot be less than 2 minutes'
);
});
test('it throws if the "importTimeout" value is greater than 1 hour', () => {
const mock: ConfigType = {
...getConfigMockDecoded(),
importTimeout: moment.duration(1, 'hour').add(1, 'second'),
};
expect(() => ConfigSchema.validate(mock)).toThrow(
'[importTimeout]: duration cannot be greater than 30 minutes'
);
});
});

10
x-pack/plugins/lists/server/config.ts
View file

@ -9,6 +9,16 @@ import { TypeOf, schema } from '@kbn/config-schema';
export const ConfigSchema = schema.object({
enabled: schema.boolean({ defaultValue: true }),
importBufferSize: schema.number({ defaultValue: 1000, min: 1 }),
importTimeout: schema.duration({
defaultValue: '5m',
validate: (value) => {
if (value.asMinutes() < 2) {
throw new Error('duration cannot be less than 2 minutes');
} else if (value.asMinutes() > 30) {
throw new Error('duration cannot be greater than 30 minutes');
}
},
}),
listIndex: schema.string({ defaultValue: '.lists' }),
listItemIndex: schema.string({ defaultValue: '.items' }),
maxImportPayloadBytes: schema.number({ defaultValue: 9000000, min: 1 }),

1
x-pack/plugins/lists/server/routes/import_list_item_route.ts
View file

@ -27,6 +27,7 @@ export const importListItemRoute = (router: IRouter, config: ConfigType): void =
parse: false,
},
tags: ['access:lists-all'],
timeout: config.importTimeout.asMilliseconds(),
},
path: `${LIST_ITEM_URL}/_import`,
validate: {

2
x-pack/plugins/lists/server/services/lists/list_client.mock.ts
View file

@ -11,6 +11,7 @@ import { getListResponseMock } from '../../../common/schemas/response/list_schem
import { getCallClusterMock } from '../../../common/get_call_cluster.mock';
import {
IMPORT_BUFFER_SIZE,
IMPORT_TIMEOUT,
LIST_INDEX,
LIST_ITEM_INDEX,
MAX_IMPORT_PAYLOAD_BYTES,
@ -65,6 +66,7 @@ export const getListClientMock = (): ListClient => {
config: {
enabled: true,
importBufferSize: IMPORT_BUFFER_SIZE,
importTimeout: IMPORT_TIMEOUT,
listIndex: LIST_INDEX,
listItemIndex: LIST_ITEM_INDEX,
maxImportPayloadBytes: MAX_IMPORT_PAYLOAD_BYTES,