Simplify router for LensProxy

Signed-off-by: Sebastian Malton <sebastian@malton.name>

packages/core/src/main/lens-proxy/lens-proxy.injectable.ts

@@ -4,7 +4,7 @@
  */
 import { getInjectable } from "@ogre-tools/injectable";
 import { LensProxy } from "./lens-proxy";
-import routerInjectable from "../router/router.injectable";
+import routeRequestInjectable from "../router/route-request.injectable";
 import httpProxy from "http-proxy";
 import shellApiRequestInjectable from "./proxy-functions/shell-api-request.injectable";
 import lensProxyPortInjectable from "./lens-proxy-port.injectable";
@@ -20,7 +20,7 @@ const lensProxyInjectable = getInjectable({
   id: "lens-proxy",
 
   instantiate: (di) => new LensProxy({
-    router: di.inject(routerInjectable),
+    routeRequest: di.inject(routeRequestInjectable),
     proxy: httpProxy.createProxy(),
     kubeApiUpgradeRequest: di.inject(kubeApiUpgradeRequestInjectable),
     shellApiRequest: di.inject(shellApiRequestInjectable),

packages/core/src/main/lens-proxy/lens-proxy.ts

@@ -8,7 +8,7 @@ import https from "https";
 import type http from "http";
 import type httpProxy from "http-proxy";
 import { apiPrefix, apiKubePrefix } from "../../common/vars";
-import type { Router } from "../router/router";
+import type { RouteRequest } from "../router/route-request.injectable";
 import type { Cluster } from "../../common/cluster/cluster";
 import type { ProxyApiRequestArgs } from "./proxy-functions";
 import { getBoolean } from "../utils/parse-query";
@@ -29,7 +29,7 @@ interface Dependencies {
   kubeApiUpgradeRequest: LensProxyApiRequest;
   emitAppEvent: EmitAppEvent;
   getKubeAuthProxyServer: (cluster: Cluster) => KubeAuthProxyServer;
-  readonly router: Router;
+  routeRequest: RouteRequest;
   readonly proxy: httpProxy;
   readonly lensProxyPort: { set: (portNumber: number) => void };
   readonly contentSecurityPolicy: string;
@@ -243,6 +243,6 @@ export class LensProxy {
     }
 
     res.setHeader("Content-Security-Policy", this.dependencies.contentSecurityPolicy);
-    await this.dependencies.router.route(cluster, req, res);
+    await this.dependencies.routeRequest(cluster, req, res);
   }
 }

packages/core/src/main/router/create-handler-for-route.injectable.ts

@@ -5,42 +5,13 @@
 import { getInjectable } from "@ogre-tools/injectable";
 import type { ServerResponse } from "http";
 import loggerInjectable from "../../common/logger.injectable";
-import { object } from "@k8slens/utilities";
 import type { LensApiRequest, Route } from "./route";
 import { contentTypes } from "./router-content-types";
+import { writeServerResponseFor } from "./write-server-response";
 
 export type RouteHandler = (request: LensApiRequest<string>, response: ServerResponse) => Promise<void>;
 export type CreateHandlerForRoute = (route: Route<unknown, string>) => RouteHandler;
 
-interface LensServerResponse {
-  statusCode: number;
-  content: any;
-  headers: {
-    [name: string]: string;
-  };
-}
-
-const writeServerResponseFor = (serverResponse: ServerResponse) => ({
-  statusCode,
-  content,
-  headers,
-}: LensServerResponse) => {
-  serverResponse.statusCode = statusCode;
-
-  for (const [name, value] of object.entries(headers)) {
-    serverResponse.setHeader(name, value);
-  }
-
-  if (content instanceof Buffer) {
-    serverResponse.write(content);
-    serverResponse.end();
-  } else if (content) {
-    serverResponse.end(content);
-  } else {
-    serverResponse.end();
-  }
-};
-
 const createHandlerForRouteInjectable = getInjectable({
   id: "create-handler-for-route",
   instantiate: (di): CreateHandlerForRoute => {
@@ -53,25 +24,21 @@ const createHandlerForRouteInjectable = getInjectable({
         const result = await route.handler(request);
 
         if (!result) {
-          const mappedResult = contentTypes.txt.resultMapper({
+          writeServerResponse(contentTypes.txt.resultMapper({
             statusCode: 204,
             response: undefined,
-          });
-
-          writeServerResponse(mappedResult);
+          }));
         } else if (!result.proxy) {
           const contentType = result.contentType || contentTypes.json;
 
           writeServerResponse(contentType.resultMapper(result));
         }
       } catch(error) {
-        const mappedResult = contentTypes.txt.resultMapper({
+        logger.error(`[ROUTER]: route ${route.path}, called with ${request.path}, threw an error`, error);
+        writeServerResponse(contentTypes.txt.resultMapper({
           statusCode: 500,
           error: error ? String(error) : "unknown error",
-        });
-
-        logger.error(`[ROUTER]: route ${route.path}, called with ${request.path}, threw an error`, error);
-        writeServerResponse(mappedResult);
+        }));
       }
     };
   },

packages/core/src/main/router/parse-request.injectable.ts

@@ -1,15 +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 Subtext from "@hapi/subtext";
-
-export type ParseRequest = typeof Subtext.parse;
-
-const parseRequestInjectable = getInjectable({
-  id: "parse-http-request",
-  instantiate: () => Subtext.parse,
-});
-
-export default parseRequestInjectable;

packages/core/src/main/router/route-request.injectable.ts

@@ -0,0 +1,80 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import type { DiContainerForInjection, Injectable, InjectionToken } from "@ogre-tools/injectable";
+import { getInjectable, getInjectionToken } from "@ogre-tools/injectable";
+import type { Route } from "./route";
+import createHandlerForRouteInjectable from "./create-handler-for-route.injectable";
+import Call from "@hapi/call";
+import Subtext from "@hapi/subtext";
+import type http from "http";
+import type { Cluster } from "../../common/cluster/cluster";
+import type { ServerIncomingMessage } from "../lens-proxy/lens-proxy";
+import type { RouteHandler } from "./create-handler-for-route.injectable";
+
+export const routeInjectionToken = getInjectionToken<Route<unknown, string>>({
+  id: "route-injection-token",
+});
+
+export function getRouteInjectable<T, Path extends string>(
+  opts: Omit<Injectable<Route<T, Path>, Route<T, Path>, void>, "lifecycle" | "injectionToken">,
+) {
+  return getInjectable({
+    ...opts,
+    injectionToken: routeInjectionToken as unknown as InjectionToken<Route<T, Path>, void>,
+  });
+}
+
+export type RouteRequest = (cluster: Cluster | undefined, req: ServerIncomingMessage, res: http.ServerResponse) => Promise<boolean>;
+
+const createRouter = (di: DiContainerForInjection) => {
+  const routes = di.injectMany(routeInjectionToken);
+  const createHandlerForRoute = di.inject(createHandlerForRouteInjectable);
+  const router = new Call.Router<RouteHandler>();
+
+  for (const route of routes) {
+    router.add({ method: route.method, path: route.path }, createHandlerForRoute(route));
+  }
+
+  return router;
+};
+
+const routeRequestInjectable = getInjectable({
+  id: "route-request",
+  instantiate: (di): RouteRequest => {
+    const router = createRouter(di);
+
+    return async (cluster, req, res) => {
+      const url = new URL(req.url, "https://localhost");
+      const path = url.pathname;
+      const method = req.method.toLowerCase();
+      const matchingRoute = router.route(method, path);
+
+      if (matchingRoute instanceof Error) {
+        return false;
+      }
+
+      const { payload } = await Subtext.parse(req, null, {
+        parse: true,
+        output: "data",
+      });
+
+      await matchingRoute.route({
+        cluster,
+        path: url.pathname,
+        raw: {
+          req, res,
+        },
+        query: url.searchParams,
+        payload,
+        params: matchingRoute.params,
+        getHeader: (key) => req.headers[key.toLowerCase()],
+      }, res);
+
+      return true;
+    };
+  },
+});
+
+export default routeRequestInjectable;

packages/core/src/main/router/route.ts

@@ -35,6 +35,7 @@ export interface LensApiRequest<Path extends string> {
   params: InferParamFromPath<Path>;
   cluster: Cluster | undefined;
   query: URLSearchParams;
+  getHeader: (key: string) => string | string[] | undefined;
   raw: {
     req: http.IncomingMessage;
     res: http.ServerResponse;

packages/core/src/main/router/router.injectable.ts

@@ -1,35 +0,0 @@
-/**
- * Copyright (c) OpenLens Authors. All rights reserved.
- * Licensed under MIT License. See LICENSE in root directory for more information.
- */
-import type { Injectable, InjectionToken } from "@ogre-tools/injectable";
-import { getInjectable, getInjectionToken } from "@ogre-tools/injectable";
-import { Router } from "./router";
-import parseRequestInjectable from "./parse-request.injectable";
-import type { Route } from "./route";
-import createHandlerForRouteInjectable from "./create-handler-for-route.injectable";
-
-export const routeInjectionToken = getInjectionToken<Route<unknown, string>>({
-  id: "route-injection-token",
-});
-
-export function getRouteInjectable<T, Path extends string>(
-  opts: Omit<Injectable<Route<T, Path>, Route<T, Path>, void>, "lifecycle" | "injectionToken">,
-) {
-  return getInjectable({
-    ...opts,
-    injectionToken: routeInjectionToken as unknown as InjectionToken<Route<T, Path>, void>,
-  });
-}
-
-const routerInjectable = getInjectable({
-  id: "router",
-
-  instantiate: (di) => new Router({
-    parseRequest: di.inject(parseRequestInjectable),
-    routes: di.injectMany(routeInjectionToken),
-    createHandlerForRoute: di.inject(createHandlerForRouteInjectable),
-  }),
-});
-
-export default routerInjectable;

packages/core/src/main/router/router.test.ts

@@ -1,301 +0,0 @@
-/**
- * Copyright (c) OpenLens Authors. All rights reserved.
- * Licensed under MIT License. See LICENSE in root directory for more information.
- */
-
-import routerInjectable, { routeInjectionToken } from "./router.injectable";
-import { getDiForUnitTesting } from "../getDiForUnitTesting";
-import type { Router } from "./router";
-import type { Cluster } from "../../common/cluster/cluster";
-import { Request } from "mock-http";
-import { getInjectable } from "@ogre-tools/injectable";
-import type { AsyncFnMock } from "@async-fn/jest";
-import asyncFn from "@async-fn/jest";
-import parseRequestInjectable from "./parse-request.injectable";
-import { contentTypes } from "./router-content-types";
-import directoryForUserDataInjectable from "../../common/app-paths/directory-for-user-data/directory-for-user-data.injectable";
-import type { Route } from "./route";
-import type { SetRequired } from "type-fest";
-import normalizedPlatformInjectable from "../../common/vars/normalized-platform.injectable";
-import kubectlBinaryNameInjectable from "../kubectl/binary-name.injectable";
-import kubectlDownloadingNormalizedArchInjectable from "../kubectl/normalized-arch.injectable";
-import { runInAction } from "mobx";
-
-describe("router", () => {
-  let router: Router;
-  let routeHandlerMock: AsyncFnMock<() => any>;
-
-  beforeEach(async () => {
-    routeHandlerMock = asyncFn();
-
-    const di = getDiForUnitTesting();
-
-    di.override(parseRequestInjectable, () => () => Promise.resolve({
-      payload: "some-payload",
-      mime: "some-mime",
-    }));
-    di.override(directoryForUserDataInjectable, () => "/some-directory-for-user-data");
-    di.override(kubectlBinaryNameInjectable, () => "kubectl");
-    di.override(kubectlDownloadingNormalizedArchInjectable, () => "amd64");
-    di.override(normalizedPlatformInjectable, () => "darwin");
-
-    const injectable = getInjectable({
-      id: "some-route",
-
-      instantiate: () => ({
-        method: "get",
-        path: "/some-path",
-        handler: routeHandlerMock,
-      } as Route<any, string>),
-
-      injectionToken: routeInjectionToken,
-    });
-
-    runInAction(() => {
-      di.register(injectable);
-    });
-
-    router = di.inject(routerInjectable);
-  });
-
-  describe("when navigating to the route", () => {
-    let actualPromise: Promise<boolean>;
-    let clusterStub: Cluster;
-    let requestStub: SetRequired<Request, "url" | "method">;
-    let responseStub: any;
-
-    beforeEach(() => {
-      requestStub = new Request({
-        url: "/some-path",
-        method: "get",
-        headers: {
-          "content-type": "application/json",
-        },
-      }) as SetRequired<Request, "url" | "method">;
-
-      responseStub = { end: jest.fn(), setHeader: jest.fn(), write: jest.fn(), statusCode: undefined };
-
-      clusterStub = {} as Cluster;
-
-      actualPromise = router.route(clusterStub, requestStub, responseStub);
-    });
-
-    it("calls handler with the request", () => {
-      expect(routeHandlerMock).toHaveBeenCalledWith({
-        cluster: clusterStub,
-        params: {},
-        path: "/some-path",
-        payload: "some-payload",
-        query: expect.any(URLSearchParams),
-        raw: { req: requestStub, res: responseStub },
-      });
-    });
-
-    it("given no content-type is specified, when handler resolves, resolves with JSON", async () => {
-      await routeHandlerMock.resolve({ response: "some-response-from-route-handler" });
-
-      await actualPromise;
-
-      expect(responseStub.setHeader.mock.calls).toEqual([
-        ["Content-Type", "application/json"],
-      ]);
-    });
-
-    it("given JSON content-type is specified, when handler resolves with object, resolves with JSON", async () => {
-      await routeHandlerMock.resolve({ response: { some: "object" }});
-
-      await actualPromise;
-
-      expect(responseStub.end).toHaveBeenCalledWith('{"some":"object"}');
-    });
-
-    describe("when handler resolves without any result", () => {
-      beforeEach(async () => {
-        await routeHandlerMock.resolve(undefined);
-
-        await actualPromise;
-      });
-
-      it("resolves as plain text", () => {
-        expect(responseStub.setHeader.mock.calls).toEqual([["Content-Type", "text/plain"]]);
-      });
-
-      it("resolves with status code for no content", async () => {
-        expect(responseStub.statusCode).toBe(204);
-      });
-
-      it("resolves without content", async () => {
-        expect(responseStub.end.mock.calls).toEqual([[]]);
-      });
-    });
-
-    describe("when handler rejects", () => {
-      beforeEach(async () => {
-        await routeHandlerMock.reject(new Error("some-error"));
-
-        await actualPromise;
-      });
-
-      it("resolves as plain text", () => {
-        expect(responseStub.setHeader.mock.calls).toEqual([["Content-Type", "text/plain"]]);
-      });
-
-      it('resolves with "500" status code', () => {
-        expect(responseStub.statusCode).toBe(500);
-      });
-
-      it("resolves with error", () => {
-        expect(responseStub.end).toHaveBeenCalledWith("Error: some-error");
-      });
-    });
-
-    [
-      { contentType: "text/plain", contentTypeObject: contentTypes.txt },
-      { contentType: "application/json", contentTypeObject: contentTypes.json },
-      { contentType: "text/html", contentTypeObject: contentTypes.html },
-      { contentType: "text/css", contentTypeObject: contentTypes.css },
-      { contentType: "image/gif", contentTypeObject: contentTypes.gif },
-      { contentType: "image/jpeg", contentTypeObject: contentTypes.jpg },
-      { contentType: "image/png", contentTypeObject: contentTypes.png },
-      { contentType: "image/svg+xml", contentTypeObject: contentTypes.svg },
-      { contentType: "application/javascript", contentTypeObject: contentTypes.js },
-      { contentType: "font/woff2", contentTypeObject: contentTypes.woff2 },
-      { contentType: "font/ttf", contentTypeObject: contentTypes.ttf },
-    ].forEach(scenario => {
-      describe(`given content type is "${scenario.contentType}", when handler resolves with response`, () => {
-        beforeEach(async () => {
-          await routeHandlerMock.resolve({ response: "some-response", contentType: scenario.contentTypeObject });
-
-          await actualPromise;
-        });
-
-        it("has content type specific headers", () => {
-          expect(responseStub.setHeader.mock.calls).toEqual([
-            ["Content-Type", scenario.contentType],
-          ]);
-        });
-
-        it("defaults to successful status code", () => {
-          expect(responseStub.statusCode).toBe(200);
-        });
-
-        it("has response as body", () => {
-          expect(responseStub.end).toHaveBeenCalledWith("some-response");
-        });
-      });
-
-      it(`given content type is "${scenario.contentType}", when handler resolves with success and custom status code, defaults to "200" as status code`, async () => {
-        await routeHandlerMock.resolve({
-          response: "some-response",
-          statusCode: 204,
-          contentType: scenario.contentTypeObject,
-        });
-
-        await actualPromise;
-
-        expect(responseStub.statusCode).toBe(204);
-      });
-
-      it(`given content type is "${scenario.contentType}", when handler resolves with success but without status code, defaults to "200" as status code`, async () => {
-        await routeHandlerMock.resolve({
-          response: "some-response",
-          contentType: scenario.contentTypeObject,
-        });
-
-        await actualPromise;
-
-        expect(responseStub.statusCode).toBe(200);
-      });
-
-
-      it(`given content type is "${scenario.contentType}", when handler resolves without response, has no body`, async () => {
-        await routeHandlerMock.resolve({
-          response: undefined,
-          contentType: scenario.contentTypeObject,
-        });
-
-        await actualPromise;
-
-        expect(responseStub.end.mock.calls).toEqual([[]]);
-      });
-
-      it(`given content type is "${scenario.contentType}", when handler resolves with error, has error as body`, async () => {
-        await routeHandlerMock.resolve({
-          error: "some-error",
-          contentType: scenario.contentTypeObject,
-        });
-
-        await actualPromise;
-
-        expect(responseStub.end).toHaveBeenCalledWith("some-error");
-      });
-
-      it(`given content type is "${scenario.contentType}", when handler resolves with error and status code, has custom status code`, async () => {
-        await routeHandlerMock.resolve({
-          error: "some-error",
-          statusCode: 414,
-          contentType: scenario.contentTypeObject,
-        });
-
-        await actualPromise;
-
-        expect(responseStub.statusCode).toBe(414);
-      });
-
-      it(`given content type is "${scenario.contentType}", when handler resolves with error but without status code, defaults to "400" as status code`, async () => {
-        await routeHandlerMock.resolve({
-          error: "some-error",
-          contentType: scenario.contentTypeObject,
-        });
-
-        await actualPromise;
-
-        expect(responseStub.statusCode).toBe(400);
-      });
-
-      it(`given content type is "${scenario.contentType}", when handler resolves custom headers, resolves with content type specific headers and custom headers`, async () => {
-        await routeHandlerMock.resolve({
-          response: "irrelevant",
-
-          headers: {
-            "Content-Type": "some-content-type-to-be-overridden",
-            "Some-Header": "some-header-value",
-          },
-
-          contentType: scenario.contentTypeObject,
-        });
-
-        await actualPromise;
-
-        expect(responseStub.setHeader.mock.calls).toEqual([
-          ["Content-Type", scenario.contentType],
-          ["Some-Header", "some-header-value"],
-        ]);
-
-      });
-
-      describe(`given content type is "${scenario.contentType}", when handler resolves with binary content`, () => {
-        let responseBufferStub: Buffer;
-
-        beforeEach(async () => {
-          responseBufferStub = Buffer.from("some-binary-content");
-
-          await routeHandlerMock.resolve({
-            response: responseBufferStub,
-            contentType: scenario.contentTypeObject,
-          });
-
-          await actualPromise;
-        });
-
-        it("writes binary content to response", () => {
-          expect(responseStub.write).toHaveBeenCalledWith(responseBufferStub);
-        });
-
-        it("does not end with the response", () => {
-          expect(responseStub.end.mock.calls[0]).toEqual([]);
-        });
-      });
-    });
-  });
-});

packages/core/src/main/router/router.ts

@@ -1,72 +0,0 @@
-/**
- * Copyright (c) OpenLens Authors. All rights reserved.
- * Licensed under MIT License. See LICENSE in root directory for more information.
- */
-
-import Call from "@hapi/call";
-import type http from "http";
-import type { Cluster } from "../../common/cluster/cluster";
-import type { LensApiRequest, Route } from "./route";
-import type { ServerIncomingMessage } from "../lens-proxy/lens-proxy";
-import type { ParseRequest } from "./parse-request.injectable";
-import type { CreateHandlerForRoute, RouteHandler } from "./create-handler-for-route.injectable";
-
-export interface RouterRequestOpts {
-  req: http.IncomingMessage;
-  res: http.ServerResponse;
-  cluster: Cluster | undefined;
-  params: Partial<Record<string, string>>;
-  url: URL;
-}
-
-interface Dependencies {
-  parseRequest: ParseRequest;
-  createHandlerForRoute: CreateHandlerForRoute;
-  readonly routes: Route<unknown, string>[];
-}
-
-export class Router {
-  private readonly router = new Call.Router<RouteHandler>();
-
-  constructor(private readonly dependencies: Dependencies) {
-    for (const route of this.dependencies.routes) {
-      this.router.add({ method: route.method, path: route.path }, this.dependencies.createHandlerForRoute(route));
-    }
-  }
-
-  public async route(cluster: Cluster | undefined, req: ServerIncomingMessage, res: http.ServerResponse): Promise<boolean> {
-    const url = new URL(req.url, "http://localhost");
-    const path = url.pathname;
-    const method = req.method.toLowerCase();
-    const matchingRoute = this.router.route(method, path);
-
-    if (matchingRoute instanceof Error) {
-      return false;
-    }
-
-    const request = await this.getRequest({ req, res, cluster, url, params: matchingRoute.params });
-
-    await matchingRoute.route(request, res);
-
-    return true;
-  }
-
-  protected async getRequest(opts: RouterRequestOpts): Promise<LensApiRequest<string>> {
-    const { req, res, url, cluster, params } = opts;
-    const { payload } = await this.dependencies.parseRequest(req, null, {
-      parse: true,
-      output: "data",
-    });
-
-    return {
-      cluster,
-      path: url.pathname,
-      raw: {
-        req, res,
-      },
-      query: url.searchParams,
-      payload,
-      params,
-    };
-  }
-}

packages/core/src/main/router/write-server-response.ts

@@ -0,0 +1,36 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+
+import type { ServerResponse } from "http";
+import { object } from "@k8slens/utilities";
+
+export interface LensServerResponse {
+  statusCode: number;
+  content: any;
+  headers: {
+    [name: string]: string;
+  };
+}
+
+export const writeServerResponseFor = (serverResponse: ServerResponse) => ({
+  statusCode,
+  content,
+  headers,
+}: LensServerResponse) => {
+  serverResponse.statusCode = statusCode;
+
+  for (const [name, value] of object.entries(headers)) {
+    serverResponse.setHeader(name, value);
+  }
+
+  if (content instanceof Buffer) {
+    serverResponse.write(content);
+    serverResponse.end();
+  } else if (content) {
+    serverResponse.end(content);
+  } else {
+    serverResponse.end();
+  }
+};