From 6b42af7e8b428e5eb5f80a8d7100132fe81e93e3 Mon Sep 17 00:00:00 2001 From: John Schulz Date: Mon, 13 Jul 2020 17:17:23 -0400 Subject: [PATCH 1/5] Use updated onPreAuth from Platform --- .../ingest_manager/common/constants/routes.ts | 2 + .../ingest_manager/server/constants/index.ts | 1 + .../plugins/ingest_manager/server/plugin.ts | 4 ++ .../server/routes/agent/index.ts | 8 +-- .../server/routes/global_interceptors.ts | 63 +++++++++++++++++++ .../ingest_manager/server/routes/index.ts | 1 + 6 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 x-pack/plugins/ingest_manager/server/routes/global_interceptors.ts diff --git a/x-pack/plugins/ingest_manager/common/constants/routes.ts b/x-pack/plugins/ingest_manager/common/constants/routes.ts index 7c3b5a198571c..94265c3920922 100644 --- a/x-pack/plugins/ingest_manager/common/constants/routes.ts +++ b/x-pack/plugins/ingest_manager/common/constants/routes.ts @@ -11,6 +11,8 @@ export const PACKAGE_CONFIG_API_ROOT = `${API_ROOT}/package_configs`; export const AGENT_CONFIG_API_ROOT = `${API_ROOT}/agent_configs`; export const FLEET_API_ROOT = `${API_ROOT}/fleet`; +export const LIMITED_CONCURRENCY_ROUTE_TAG = 'ingest:limited-concurrency'; + // EPM API routes const EPM_PACKAGES_MANY = `${EPM_API_ROOT}/packages`; const EPM_PACKAGES_ONE = `${EPM_PACKAGES_MANY}/{pkgkey}`; diff --git a/x-pack/plugins/ingest_manager/server/constants/index.ts b/x-pack/plugins/ingest_manager/server/constants/index.ts index d3c074ff2e8d0..ce81736f2e84f 100644 --- a/x-pack/plugins/ingest_manager/server/constants/index.ts +++ b/x-pack/plugins/ingest_manager/server/constants/index.ts @@ -15,6 +15,7 @@ export { AGENT_UPDATE_ACTIONS_INTERVAL_MS, INDEX_PATTERN_PLACEHOLDER_SUFFIX, // Routes + LIMITED_CONCURRENCY_ROUTE_TAG, PLUGIN_ID, EPM_API_ROUTES, DATA_STREAM_API_ROUTES, diff --git a/x-pack/plugins/ingest_manager/server/plugin.ts b/x-pack/plugins/ingest_manager/server/plugin.ts index d1adbd8b2f65d..db6bccd8c134c 100644 --- a/x-pack/plugins/ingest_manager/server/plugin.ts +++ b/x-pack/plugins/ingest_manager/server/plugin.ts @@ -34,6 +34,7 @@ import { } from './constants'; import { registerSavedObjects, registerEncryptedSavedObjects } from './saved_objects'; import { + preAuthHandler, registerEPMRoutes, registerPackageConfigRoutes, registerDataStreamRoutes, @@ -231,6 +232,9 @@ export class IngestManagerPlugin ); } } else { + // we currently only use this global interceptor if fleet is enabled + // since it would run this func on *every* req (other plugins, CSS, etc) + this.httpSetup.registerOnPreAuth(preAuthHandler); registerAgentRoutes(router); registerEnrollmentApiKeyRoutes(router); registerInstallScriptRoutes({ diff --git a/x-pack/plugins/ingest_manager/server/routes/agent/index.ts b/x-pack/plugins/ingest_manager/server/routes/agent/index.ts index d7eec50eac3cf..8f79d1dfedea9 100644 --- a/x-pack/plugins/ingest_manager/server/routes/agent/index.ts +++ b/x-pack/plugins/ingest_manager/server/routes/agent/index.ts @@ -10,7 +10,7 @@ */ import { IRouter } from 'src/core/server'; -import { PLUGIN_ID, AGENT_API_ROUTES } from '../../constants'; +import { PLUGIN_ID, AGENT_API_ROUTES, LIMITED_CONCURRENCY_ROUTE_TAG } from '../../constants'; import { GetAgentsRequestSchema, GetOneAgentRequestSchema, @@ -85,7 +85,7 @@ export const registerRoutes = (router: IRouter) => { { path: AGENT_API_ROUTES.CHECKIN_PATTERN, validate: PostAgentCheckinRequestSchema, - options: { tags: [] }, + options: { tags: [LIMITED_CONCURRENCY_ROUTE_TAG] }, }, postAgentCheckinHandler ); @@ -95,7 +95,7 @@ export const registerRoutes = (router: IRouter) => { { path: AGENT_API_ROUTES.ENROLL_PATTERN, validate: PostAgentEnrollRequestSchema, - options: { tags: [] }, + options: { tags: [LIMITED_CONCURRENCY_ROUTE_TAG] }, }, postAgentEnrollHandler ); @@ -105,7 +105,7 @@ export const registerRoutes = (router: IRouter) => { { path: AGENT_API_ROUTES.ACKS_PATTERN, validate: PostAgentAcksRequestSchema, - options: { tags: [] }, + options: { tags: [LIMITED_CONCURRENCY_ROUTE_TAG] }, }, postAgentAcksHandlerBuilder({ acknowledgeAgentActions: AgentService.acknowledgeAgentActions, diff --git a/x-pack/plugins/ingest_manager/server/routes/global_interceptors.ts b/x-pack/plugins/ingest_manager/server/routes/global_interceptors.ts new file mode 100644 index 0000000000000..a2e0cc446827e --- /dev/null +++ b/x-pack/plugins/ingest_manager/server/routes/global_interceptors.ts @@ -0,0 +1,63 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { KibanaRequest, LifecycleResponseFactory, OnPreAuthToolkit } from 'kibana/server'; +import { LIMITED_CONCURRENCY_ROUTE_TAG } from '../../common'; + +class MaxCounter { + constructor(private readonly max: number = 1) {} + private counter = 0; + valueOf() { + return this.counter; + } + increase() { + if (this.counter < this.max) { + this.counter += 1; + } + } + decrease() { + this.counter += 1; + } + lessThanMax() { + return this.counter < this.max; + } +} + +function shouldHandleRequest(request: KibanaRequest) { + const tags = request.route.options.tags; + return tags.includes(LIMITED_CONCURRENCY_ROUTE_TAG); +} + +const LIMITED_CONCURRENCY_MAX_REQUESTS = 250; +const counter = new MaxCounter(LIMITED_CONCURRENCY_MAX_REQUESTS); + +export function preAuthHandler( + request: KibanaRequest, + response: LifecycleResponseFactory, + toolkit: OnPreAuthToolkit +) { + if (!shouldHandleRequest(request)) { + return toolkit.next(); + } + + if (!counter.lessThanMax()) { + return response.customError({ + body: 'Too Many Agents', + statusCode: 503, + headers: { + 'Retry-After': '30', + }, + }); + } + + counter.increase(); + + // requests.events.aborted$ has a bug where it's fired even when the request completes... + // we can take advantage of this bug just for load testing... + request.events.aborted$.toPromise().then(() => counter.decrease()); + + return toolkit.next(); +} diff --git a/x-pack/plugins/ingest_manager/server/routes/index.ts b/x-pack/plugins/ingest_manager/server/routes/index.ts index f6b4439d8bef1..076ea8c07b063 100644 --- a/x-pack/plugins/ingest_manager/server/routes/index.ts +++ b/x-pack/plugins/ingest_manager/server/routes/index.ts @@ -14,3 +14,4 @@ export { registerRoutes as registerInstallScriptRoutes } from './install_script' export { registerRoutes as registerOutputRoutes } from './output'; export { registerRoutes as registerSettingsRoutes } from './settings'; export { registerRoutes as registerAppRoutes } from './app'; +export { preAuthHandler } from './global_interceptors'; From 8174114aec75a3e8a528fdc8dcc04e1c75adff53 Mon Sep 17 00:00:00 2001 From: John Schulz Date: Tue, 14 Jul 2020 11:04:14 -0400 Subject: [PATCH 2/5] Add config flag. Increase default value. --- .../ingest_manager/common/types/index.ts | 1 + x-pack/plugins/ingest_manager/server/index.ts | 1 + .../plugins/ingest_manager/server/plugin.ts | 4 +- .../server/routes/global_interceptors.ts | 63 ---------------- .../ingest_manager/server/routes/index.ts | 2 +- .../server/routes/limited_concurrency.ts | 72 +++++++++++++++++++ 6 files changed, 77 insertions(+), 66 deletions(-) delete mode 100644 x-pack/plugins/ingest_manager/server/routes/global_interceptors.ts create mode 100644 x-pack/plugins/ingest_manager/server/routes/limited_concurrency.ts diff --git a/x-pack/plugins/ingest_manager/common/types/index.ts b/x-pack/plugins/ingest_manager/common/types/index.ts index 0fce5cfa6226f..d7edc04a35799 100644 --- a/x-pack/plugins/ingest_manager/common/types/index.ts +++ b/x-pack/plugins/ingest_manager/common/types/index.ts @@ -13,6 +13,7 @@ export interface IngestManagerConfigType { enabled: boolean; tlsCheckDisabled: boolean; pollingRequestTimeout: number; + maxConcurrentConnections: number; kibana: { host?: string; ca_sha256?: string; diff --git a/x-pack/plugins/ingest_manager/server/index.ts b/x-pack/plugins/ingest_manager/server/index.ts index 1823cc3561693..10f2097a47e78 100644 --- a/x-pack/plugins/ingest_manager/server/index.ts +++ b/x-pack/plugins/ingest_manager/server/index.ts @@ -26,6 +26,7 @@ export const config = { enabled: schema.boolean({ defaultValue: true }), tlsCheckDisabled: schema.boolean({ defaultValue: false }), pollingRequestTimeout: schema.number({ defaultValue: 60000 }), + maxConcurrentConnections: schema.number({ defaultValue: 750 }), kibana: schema.object({ host: schema.maybe(schema.string()), ca_sha256: schema.maybe(schema.string()), diff --git a/x-pack/plugins/ingest_manager/server/plugin.ts b/x-pack/plugins/ingest_manager/server/plugin.ts index be96ea3fc4ef8..69af475886bb9 100644 --- a/x-pack/plugins/ingest_manager/server/plugin.ts +++ b/x-pack/plugins/ingest_manager/server/plugin.ts @@ -34,7 +34,7 @@ import { } from './constants'; import { registerSavedObjects, registerEncryptedSavedObjects } from './saved_objects'; import { - preAuthHandler, + registerLimitedConcurrencyRoutes, registerEPMRoutes, registerPackageConfigRoutes, registerDataStreamRoutes, @@ -231,7 +231,7 @@ export class IngestManagerPlugin } else { // we currently only use this global interceptor if fleet is enabled // since it would run this func on *every* req (other plugins, CSS, etc) - this.httpSetup.registerOnPreAuth(preAuthHandler); + registerLimitedConcurrencyRoutes(core, config); registerAgentRoutes(router); registerEnrollmentApiKeyRoutes(router); registerInstallScriptRoutes({ diff --git a/x-pack/plugins/ingest_manager/server/routes/global_interceptors.ts b/x-pack/plugins/ingest_manager/server/routes/global_interceptors.ts deleted file mode 100644 index a2e0cc446827e..0000000000000 --- a/x-pack/plugins/ingest_manager/server/routes/global_interceptors.ts +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { KibanaRequest, LifecycleResponseFactory, OnPreAuthToolkit } from 'kibana/server'; -import { LIMITED_CONCURRENCY_ROUTE_TAG } from '../../common'; - -class MaxCounter { - constructor(private readonly max: number = 1) {} - private counter = 0; - valueOf() { - return this.counter; - } - increase() { - if (this.counter < this.max) { - this.counter += 1; - } - } - decrease() { - this.counter += 1; - } - lessThanMax() { - return this.counter < this.max; - } -} - -function shouldHandleRequest(request: KibanaRequest) { - const tags = request.route.options.tags; - return tags.includes(LIMITED_CONCURRENCY_ROUTE_TAG); -} - -const LIMITED_CONCURRENCY_MAX_REQUESTS = 250; -const counter = new MaxCounter(LIMITED_CONCURRENCY_MAX_REQUESTS); - -export function preAuthHandler( - request: KibanaRequest, - response: LifecycleResponseFactory, - toolkit: OnPreAuthToolkit -) { - if (!shouldHandleRequest(request)) { - return toolkit.next(); - } - - if (!counter.lessThanMax()) { - return response.customError({ - body: 'Too Many Agents', - statusCode: 503, - headers: { - 'Retry-After': '30', - }, - }); - } - - counter.increase(); - - // requests.events.aborted$ has a bug where it's fired even when the request completes... - // we can take advantage of this bug just for load testing... - request.events.aborted$.toPromise().then(() => counter.decrease()); - - return toolkit.next(); -} diff --git a/x-pack/plugins/ingest_manager/server/routes/index.ts b/x-pack/plugins/ingest_manager/server/routes/index.ts index 076ea8c07b063..87be3a80cea96 100644 --- a/x-pack/plugins/ingest_manager/server/routes/index.ts +++ b/x-pack/plugins/ingest_manager/server/routes/index.ts @@ -14,4 +14,4 @@ export { registerRoutes as registerInstallScriptRoutes } from './install_script' export { registerRoutes as registerOutputRoutes } from './output'; export { registerRoutes as registerSettingsRoutes } from './settings'; export { registerRoutes as registerAppRoutes } from './app'; -export { preAuthHandler } from './global_interceptors'; +export { registerLimitedConcurrencyRoutes } from './limited_concurrency'; diff --git a/x-pack/plugins/ingest_manager/server/routes/limited_concurrency.ts b/x-pack/plugins/ingest_manager/server/routes/limited_concurrency.ts new file mode 100644 index 0000000000000..ec8e2f6c8d436 --- /dev/null +++ b/x-pack/plugins/ingest_manager/server/routes/limited_concurrency.ts @@ -0,0 +1,72 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { + CoreSetup, + KibanaRequest, + LifecycleResponseFactory, + OnPreAuthToolkit, +} from 'kibana/server'; +import { LIMITED_CONCURRENCY_ROUTE_TAG } from '../../common'; +import { IngestManagerConfigType } from '../index'; +class MaxCounter { + constructor(private readonly max: number = 1) {} + private counter = 0; + valueOf() { + return this.counter; + } + increase() { + if (this.counter < this.max) { + this.counter += 1; + } + } + decrease() { + if (this.counter > 0) { + this.counter -= 1; + } + } + lessThanMax() { + return this.counter < this.max; + } +} + +function shouldHandleRequest(request: KibanaRequest) { + const tags = request.route.options.tags; + return tags.includes(LIMITED_CONCURRENCY_ROUTE_TAG); +} + +export function registerLimitedConcurrencyRoutes(core: CoreSetup, config: IngestManagerConfigType) { + const max = config.fleet.maxConcurrentConnections; + if (!max) return; + + const counter = new MaxCounter(max); + core.http.registerOnPreAuth(function preAuthHandler( + request: KibanaRequest, + response: LifecycleResponseFactory, + toolkit: OnPreAuthToolkit + ) { + if (!shouldHandleRequest(request)) { + return toolkit.next(); + } + + if (!counter.lessThanMax()) { + return response.customError({ + body: 'Too Many Requests', + statusCode: 429, + }); + } + + counter.increase(); + + // requests.events.aborted$ has a bug (but has test which explicitly verifies) where it's fired even when the request completes + // https://github.com/elastic/kibana/pull/70495#issuecomment-656288766 + request.events.aborted$.toPromise().then(() => { + counter.decrease(); + }); + + return toolkit.next(); + }); +} From 7cc7f2d3bd4d273edad96487a91aac12d5690b9f Mon Sep 17 00:00:00 2001 From: John Schulz Date: Tue, 14 Jul 2020 13:26:09 -0400 Subject: [PATCH 3/5] Set max connections flag default to 0 (disabled) --- x-pack/plugins/ingest_manager/server/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/ingest_manager/server/index.ts b/x-pack/plugins/ingest_manager/server/index.ts index 0fa055b9740bb..6c72218abc531 100644 --- a/x-pack/plugins/ingest_manager/server/index.ts +++ b/x-pack/plugins/ingest_manager/server/index.ts @@ -26,7 +26,7 @@ export const config = { enabled: schema.boolean({ defaultValue: true }), tlsCheckDisabled: schema.boolean({ defaultValue: false }), pollingRequestTimeout: schema.number({ defaultValue: 60000 }), - maxConcurrentConnections: schema.number({ defaultValue: 750 }), + maxConcurrentConnections: schema.number({ defaultValue: 0 }), kibana: schema.object({ host: schema.maybe(schema.string()), ca_sha256: schema.maybe(schema.string()), From aba7a45f2da243daaa9a30dedf09369a77653c24 Mon Sep 17 00:00:00 2001 From: John Schulz Date: Tue, 14 Jul 2020 13:32:04 -0400 Subject: [PATCH 4/5] Don't use limiting logic on checkin route --- x-pack/plugins/ingest_manager/server/routes/agent/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/ingest_manager/server/routes/agent/index.ts b/x-pack/plugins/ingest_manager/server/routes/agent/index.ts index 8f79d1dfedea9..b85d96186f233 100644 --- a/x-pack/plugins/ingest_manager/server/routes/agent/index.ts +++ b/x-pack/plugins/ingest_manager/server/routes/agent/index.ts @@ -85,7 +85,7 @@ export const registerRoutes = (router: IRouter) => { { path: AGENT_API_ROUTES.CHECKIN_PATTERN, validate: PostAgentCheckinRequestSchema, - options: { tags: [LIMITED_CONCURRENCY_ROUTE_TAG] }, + options: { tags: [] }, }, postAgentCheckinHandler ); From b8288ae4da9e0b5236c767b12c84a9da74845437 Mon Sep 17 00:00:00 2001 From: John Schulz Date: Tue, 14 Jul 2020 15:27:17 -0400 Subject: [PATCH 5/5] Confirm preAuth handler only added when max > 0 --- .../server/routes/limited_concurrency.test.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 x-pack/plugins/ingest_manager/server/routes/limited_concurrency.test.ts diff --git a/x-pack/plugins/ingest_manager/server/routes/limited_concurrency.test.ts b/x-pack/plugins/ingest_manager/server/routes/limited_concurrency.test.ts new file mode 100644 index 0000000000000..a0bb8e9b86fbb --- /dev/null +++ b/x-pack/plugins/ingest_manager/server/routes/limited_concurrency.test.ts @@ -0,0 +1,35 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { coreMock } from 'src/core/server/mocks'; +import { registerLimitedConcurrencyRoutes } from './limited_concurrency'; +import { IngestManagerConfigType } from '../index'; + +describe('registerLimitedConcurrencyRoutes', () => { + test(`doesn't call registerOnPreAuth if maxConcurrentConnections is 0`, async () => { + const mockSetup = coreMock.createSetup(); + const mockConfig = { fleet: { maxConcurrentConnections: 0 } } as IngestManagerConfigType; + registerLimitedConcurrencyRoutes(mockSetup, mockConfig); + + expect(mockSetup.http.registerOnPreAuth).not.toHaveBeenCalled(); + }); + + test(`calls registerOnPreAuth once if maxConcurrentConnections is 1`, async () => { + const mockSetup = coreMock.createSetup(); + const mockConfig = { fleet: { maxConcurrentConnections: 1 } } as IngestManagerConfigType; + registerLimitedConcurrencyRoutes(mockSetup, mockConfig); + + expect(mockSetup.http.registerOnPreAuth).toHaveBeenCalledTimes(1); + }); + + test(`calls registerOnPreAuth once if maxConcurrentConnections is 1000`, async () => { + const mockSetup = coreMock.createSetup(); + const mockConfig = { fleet: { maxConcurrentConnections: 1000 } } as IngestManagerConfigType; + registerLimitedConcurrencyRoutes(mockSetup, mockConfig); + + expect(mockSetup.http.registerOnPreAuth).toHaveBeenCalledTimes(1); + }); +});