From b6050dea4ef92809afdd86f077431b0d45ea941c Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Tue, 21 Mar 2023 15:56:54 -0400 Subject: [PATCH] Move httpProxy server to injectables Signed-off-by: Sebastian Malton --- .../core/src/main/__test__/lens-proxy.test.ts | 2 +- .../handle-route-request.injectable.ts | 6 +- ...{is-long-running-request.ts => helpers.ts} | 5 ++ .../main/lens-proxy/lens-proxy.injectable.ts | 6 +- .../core/src/main/lens-proxy/lens-proxy.ts | 73 ++----------------- .../src/main/lens-proxy/proxy.injectable.ts | 13 ---- .../lens-proxy/proxy/http-proxy.injectable.ts | 22 ++++++ .../lens-proxy/proxy/on-error.injectable.ts | 57 +++++++++++++++ .../proxy/on-proxy-res.injectable.ts | 25 +++++++ .../lens-proxy/proxy/raw-proxy.injectable.ts | 15 ++++ .../main/lens-proxy/proxy/retry.injectable.ts | 37 ++++++++++ 11 files changed, 172 insertions(+), 89 deletions(-) rename packages/core/src/main/lens-proxy/{is-long-running-request.ts => helpers.ts} (76%) delete mode 100644 packages/core/src/main/lens-proxy/proxy.injectable.ts create mode 100644 packages/core/src/main/lens-proxy/proxy/http-proxy.injectable.ts create mode 100644 packages/core/src/main/lens-proxy/proxy/on-error.injectable.ts create mode 100644 packages/core/src/main/lens-proxy/proxy/on-proxy-res.injectable.ts create mode 100644 packages/core/src/main/lens-proxy/proxy/raw-proxy.injectable.ts create mode 100644 packages/core/src/main/lens-proxy/proxy/retry.injectable.ts diff --git a/packages/core/src/main/__test__/lens-proxy.test.ts b/packages/core/src/main/__test__/lens-proxy.test.ts index 665b890d33..4642bd15e2 100644 --- a/packages/core/src/main/__test__/lens-proxy.test.ts +++ b/packages/core/src/main/__test__/lens-proxy.test.ts @@ -3,7 +3,7 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ -import { isLongRunningRequest } from "../lens-proxy/is-long-running-request"; +import { isLongRunningRequest } from "../lens-proxy/helpers"; describe("isLongRunningRequest", () => { it("returns true on watches", () => { diff --git a/packages/core/src/main/lens-proxy/handle-route-request.injectable.ts b/packages/core/src/main/lens-proxy/handle-route-request.injectable.ts index d0bbc8017b..c156428a64 100644 --- a/packages/core/src/main/lens-proxy/handle-route-request.injectable.ts +++ b/packages/core/src/main/lens-proxy/handle-route-request.injectable.ts @@ -9,9 +9,9 @@ import contentSecurityPolicyInjectable from "../../common/vars/content-security- import kubeAuthProxyServerInjectable from "../cluster/kube-auth-proxy-server.injectable"; import routeRequestInjectable from "../router/route-request.injectable"; import getClusterForRequestInjectable from "./get-cluster-for-request.injectable"; -import { isLongRunningRequest } from "./is-long-running-request"; +import { isLongRunningRequest } from "./helpers"; import type { ProxyIncomingMessage } from "./messages"; -import proxyInjectable from "./proxy.injectable"; +import rawHttpProxyInjectable from "./proxy/raw-proxy.injectable"; export type HandleRouteRequest = (req: ProxyIncomingMessage, res: ServerResponse) => Promise; @@ -20,7 +20,7 @@ const handleRouteRequestInjectable = getInjectable({ instantiate: (di): HandleRouteRequest => { const getClusterForRequest = di.inject(getClusterForRequestInjectable); const routeRequest = di.inject(routeRequestInjectable); - const proxy = di.inject(proxyInjectable); + const proxy = di.inject(rawHttpProxyInjectable); const contentSecurityPolicy = di.inject(contentSecurityPolicyInjectable); return async (req, res) => { diff --git a/packages/core/src/main/lens-proxy/is-long-running-request.ts b/packages/core/src/main/lens-proxy/helpers.ts similarity index 76% rename from packages/core/src/main/lens-proxy/is-long-running-request.ts rename to packages/core/src/main/lens-proxy/helpers.ts index 08a96dc2b4..1fbc1b0227 100644 --- a/packages/core/src/main/lens-proxy/is-long-running-request.ts +++ b/packages/core/src/main/lens-proxy/helpers.ts @@ -2,6 +2,7 @@ * Copyright (c) OpenLens Authors. All rights reserved. * Licensed under MIT License. See LICENSE in root directory for more information. */ +import type { IncomingMessage } from "http"; import { getBoolean } from "../utils/parse-query"; const watchParam = "watch"; @@ -12,3 +13,7 @@ export function isLongRunningRequest(reqUrl: string) { return getBoolean(url.searchParams, watchParam) || getBoolean(url.searchParams, followParam); } + +export function getRequestId(req: IncomingMessage) { + return (req.headers.host ?? "") + req.url; +} diff --git a/packages/core/src/main/lens-proxy/lens-proxy.injectable.ts b/packages/core/src/main/lens-proxy/lens-proxy.injectable.ts index 2b2af200b2..265e9dbe4b 100644 --- a/packages/core/src/main/lens-proxy/lens-proxy.injectable.ts +++ b/packages/core/src/main/lens-proxy/lens-proxy.injectable.ts @@ -7,20 +7,18 @@ import { LensProxy } from "./lens-proxy"; import lensProxyPortInjectable from "./lens-proxy-port.injectable"; import emitAppEventInjectable from "../../common/app-event-bus/emit-event.injectable"; import loggerInjectable from "../../common/logger.injectable"; -import proxyInjectable from "./proxy.injectable"; -import handleRouteRequestInjectable from "./handle-route-request.injectable"; import lensProxyHttpsServerInjectable from "./https-proxy/server.injectable"; +import proxyRetryInjectable from "./proxy/retry.injectable"; const lensProxyInjectable = getInjectable({ id: "lens-proxy", instantiate: (di) => new LensProxy({ - proxy: di.inject(proxyInjectable), proxyServer: di.inject(lensProxyHttpsServerInjectable), - handleRouteRequest: di.inject(handleRouteRequestInjectable), lensProxyPort: di.inject(lensProxyPortInjectable), emitAppEvent: di.inject(emitAppEventInjectable), logger: di.inject(loggerInjectable), + proxyRetry: di.inject(proxyRetryInjectable), }), }); diff --git a/packages/core/src/main/lens-proxy/lens-proxy.ts b/packages/core/src/main/lens-proxy/lens-proxy.ts index 52c140055e..a509d0c230 100644 --- a/packages/core/src/main/lens-proxy/lens-proxy.ts +++ b/packages/core/src/main/lens-proxy/lens-proxy.ts @@ -3,18 +3,16 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ -import net from "net"; +import type net from "net"; import type https from "https"; import type http from "http"; -import type httpProxy from "http-proxy"; import type { Cluster } from "../../common/cluster/cluster"; import type { ProxyApiRequestArgs } from "./proxy-functions"; -import assert from "assert"; import type { SetRequired } from "type-fest"; import type { EmitAppEvent } from "../../common/app-event-bus/emit-event.injectable"; import type { Logger } from "../../common/logger"; import { disallowedPorts } from "./disallowed-ports"; -import type { HandleRouteRequest } from "./handle-route-request.injectable"; +import type { ProxyRetry } from "./proxy/retry.injectable"; export type GetClusterForRequest = (req: http.IncomingMessage) => Cluster | undefined; export type ServerIncomingMessage = SetRequired; @@ -22,20 +20,14 @@ export type LensProxyApiRequest = (args: ProxyApiRequestArgs) => void | Promise< interface Dependencies { emitAppEvent: EmitAppEvent; - handleRouteRequest: HandleRouteRequest; - readonly proxy: httpProxy; readonly lensProxyPort: { set: (portNumber: number) => void }; readonly logger: Logger; readonly proxyServer: https.Server; + readonly proxyRetry: ProxyRetry; } export class LensProxy { - protected closed = false; - protected retryCounters = new Map(); - - constructor(private readonly dependencies: Dependencies) { - this.configureProxy(dependencies.proxy); - } + constructor(private readonly dependencies: Dependencies) {} /** * Starts to listen on an OS provided port. Will reject if the server throws @@ -107,61 +99,6 @@ export class LensProxy { this.dependencies.logger.info("[LENS-PROXY]: Closing server"); this.dependencies.proxyServer.close(); - this.closed = true; - } - - protected configureProxy(proxy: httpProxy): httpProxy { - proxy.on("proxyRes", (proxyRes, req, res) => { - const retryCounterId = this.getRequestId(req); - - if (this.retryCounters.has(retryCounterId)) { - this.retryCounters.delete(retryCounterId); - } - - proxyRes.on("aborted", () => { // happens when proxy target aborts connection - res.end(); - }); - }); - - proxy.on("error", (error, req, res, target) => { - if (this.closed || res instanceof net.Socket) { - return; - } - - this.dependencies.logger.error(`[LENS-PROXY]: http proxy errored for cluster: ${error}`, { url: req.url }); - - if (target) { - this.dependencies.logger.debug(`Failed proxy to target: ${JSON.stringify(target, null, 2)}`); - - if (req.method === "GET" && (!res.statusCode || res.statusCode >= 500)) { - const reqId = this.getRequestId(req); - const retryCount = this.retryCounters.get(reqId) || 0; - const timeoutMs = retryCount * 250; - - if (retryCount < 20) { - this.dependencies.logger.debug(`Retrying proxy request to url: ${reqId}`); - setTimeout(() => { - this.retryCounters.set(reqId, retryCount + 1); - this.dependencies.handleRouteRequest(req as any, res) - .catch(error => this.dependencies.logger.error(`[LENS-PROXY]: failed to handle request on proxy error: ${error}`)); - }, timeoutMs); - } - } - } - - try { - res.writeHead(500).end(`Oops, something went wrong.\n${error}`); - } catch (e) { - this.dependencies.logger.error(`[LENS-PROXY]: Failed to write headers: `, e); - } - }); - - return proxy; - } - - protected getRequestId(req: http.IncomingMessage): string { - assert(req.headers.host); - - return req.headers.host + req.url; + this.dependencies.proxyRetry.close(); } } diff --git a/packages/core/src/main/lens-proxy/proxy.injectable.ts b/packages/core/src/main/lens-proxy/proxy.injectable.ts deleted file mode 100644 index 4a43c9811b..0000000000 --- a/packages/core/src/main/lens-proxy/proxy.injectable.ts +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Copyright (c) OpenLens Authors. All rights reserved. - * Licensed under MIT License. See LICENSE in root directory for more information. - */ -import { getInjectable } from "@ogre-tools/injectable"; -import { createProxy } from "http-proxy"; - -const proxyInjectable = getInjectable({ - id: "proxy", - instantiate: () => createProxy(), -}); - -export default proxyInjectable; diff --git a/packages/core/src/main/lens-proxy/proxy/http-proxy.injectable.ts b/packages/core/src/main/lens-proxy/proxy/http-proxy.injectable.ts new file mode 100644 index 0000000000..f4fce630a7 --- /dev/null +++ b/packages/core/src/main/lens-proxy/proxy/http-proxy.injectable.ts @@ -0,0 +1,22 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import onErrorInjectable from "./on-error.injectable"; +import onProxyResInjectable from "./on-proxy-res.injectable"; +import rawHttpProxyInjectable from "./raw-proxy.injectable"; + +const httpProxyInjectable = getInjectable({ + id: "http-proxy", + instantiate: (di) => { + const proxy = di.inject(rawHttpProxyInjectable); + + proxy.on("proxyRes", di.inject(onProxyResInjectable)); + proxy.on("error", di.inject(onErrorInjectable)); + + return proxy; + }, +}); + +export default httpProxyInjectable; diff --git a/packages/core/src/main/lens-proxy/proxy/on-error.injectable.ts b/packages/core/src/main/lens-proxy/proxy/on-error.injectable.ts new file mode 100644 index 0000000000..7840dc3ab2 --- /dev/null +++ b/packages/core/src/main/lens-proxy/proxy/on-error.injectable.ts @@ -0,0 +1,57 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import type { ServerResponse } from "http"; +import type { ProxyTargetUrl } from "http-proxy"; +import { Socket } from "net"; +import loggerInjectable from "../../../common/logger.injectable"; +import handleRouteRequestInjectable from "../handle-route-request.injectable"; +import { getRequestId } from "../helpers"; +import type { ProxyIncomingMessage } from "../messages"; +import proxyRetryInjectable from "./retry.injectable"; + +const onErrorInjectable = getInjectable({ + id: "on-error", + instantiate: (di) => { + const proxyRetry = di.inject(proxyRetryInjectable); + const logger = di.inject(loggerInjectable); + const handleRouteRequest = di.inject(handleRouteRequestInjectable); + + return (error: Error, req: ProxyIncomingMessage, res: ServerResponse | Socket, target?: ProxyTargetUrl) => { + if (proxyRetry.isClosed() || res instanceof Socket) { + return; + } + + logger.error(`[LENS-PROXY]: http proxy errored for cluster: ${error}`, { url: req.url }); + + if (target) { + logger.debug(`Failed proxy to target: ${JSON.stringify(target, null, 2)}`); + + if (req.method === "GET" && (!res.statusCode || res.statusCode >= 500)) { + const reqId = getRequestId(req); + const retryCount = proxyRetry.getCount(reqId); + const timeoutMs = retryCount * 250; + + if (retryCount < 20) { + logger.debug(`Retrying proxy request to url: ${reqId}`); + setTimeout(() => { + proxyRetry.incrementCount(reqId); + handleRouteRequest(req, res) + .catch(error => logger.error(`[LENS-PROXY]: failed to handle request on proxy error: ${error}`)); + }, timeoutMs); + } + } + } + + try { + res.writeHead(500).end(`Oops, something went wrong.\n${error}`); + } catch (e) { + logger.error(`[LENS-PROXY]: Failed to write headers: `, e); + } + }; + }, +}); + +export default onErrorInjectable; diff --git a/packages/core/src/main/lens-proxy/proxy/on-proxy-res.injectable.ts b/packages/core/src/main/lens-proxy/proxy/on-proxy-res.injectable.ts new file mode 100644 index 0000000000..e1e34ed4d2 --- /dev/null +++ b/packages/core/src/main/lens-proxy/proxy/on-proxy-res.injectable.ts @@ -0,0 +1,25 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import type { IncomingMessage, ServerResponse } from "http"; +import { getRequestId } from "../helpers"; +import proxyRetryInjectable from "./retry.injectable"; + +const onProxyResInjectable = getInjectable({ + id: "on-proxy-res", + instantiate: (di) => { + const proxyRetry = di.inject(proxyRetryInjectable); + + return (proxyRes: IncomingMessage, req: IncomingMessage, res: ServerResponse) => { + proxyRetry.clearCount(getRequestId(req)); + + proxyRes.on("aborted", () => { // happens when proxy target aborts connection + res.end(); + }); + }; + }, +}); + +export default onProxyResInjectable; diff --git a/packages/core/src/main/lens-proxy/proxy/raw-proxy.injectable.ts b/packages/core/src/main/lens-proxy/proxy/raw-proxy.injectable.ts new file mode 100644 index 0000000000..6f2489fe10 --- /dev/null +++ b/packages/core/src/main/lens-proxy/proxy/raw-proxy.injectable.ts @@ -0,0 +1,15 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import { createProxy } from "http-proxy"; +import type { ProxyIncomingMessage } from "../messages"; + +// NOTE: this is separate from httpProxyInjectable to fix circular dependency issues +const rawHttpProxyInjectable = getInjectable({ + id: "raw-http-proxy", + instantiate: () => createProxy(), +}); + +export default rawHttpProxyInjectable; diff --git a/packages/core/src/main/lens-proxy/proxy/retry.injectable.ts b/packages/core/src/main/lens-proxy/proxy/retry.injectable.ts new file mode 100644 index 0000000000..e97f40877c --- /dev/null +++ b/packages/core/src/main/lens-proxy/proxy/retry.injectable.ts @@ -0,0 +1,37 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; + +export interface ProxyRetry { + clearCount: (id: string) => void; + getCount: (id: string) => number; + incrementCount: (id: string) => void; + isClosed: () => boolean; + close: () => void; +} + +const proxyRetryInjectable = getInjectable({ + id: "proxy-retry", + instantiate: (): ProxyRetry => { + const counters = new Map(); + let closed = false; + + return { + clearCount: (id: string) => { + counters.delete(id); + }, + getCount: (id: string) => counters.get(id) ?? 0, + incrementCount: (id: string) => { + counters.set(id, (counters.get(id) ?? 0) + 1); + }, + isClosed: () => closed, + close: () => { + closed = true; + }, + }; + }, +}); + +export default proxyRetryInjectable;