Make routes in back-end comply to Open Closed Principle (#4859)

* Make registration of back-end routes happen using injectable Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for list charts Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for get chart Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for get chart values Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for installing chart Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for updating release Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for rollbacking release Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for listing releases Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for getting release Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for getting release values Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for getting release history Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for deleting release Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Remove HelmRoute for not being used anymore Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for applying resource Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for patching resource Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for stopping current port forward Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for getting current port forward Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Consolidate start port forward route to use injection token for registering route Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for metrics Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for getting service account Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for getting version Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Add global override for reading file to make it not required where not interesting Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Switch to using injectable to setup route for serving static files Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Make adding routes private for router Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Make routes wait until all asynchronous stuff are done Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Introduce healthy abstraction between back-end routes and route handlers Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Make response of route typed Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Fix incorrect return value of updateRelease Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Make typing of routes support synchronous values Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Make code cleaner Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Rename test for accuracy Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Add test for throwing route handler Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Remove duplicate license header Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Make mechanism of creating HTTP response an implementation detail Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Remove not needed properties from test Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Fix code style Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Fix typing and codestyle Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Fix typing error Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Fix merge conflicts Signed-off-by: Janne Savolainen <janne.savolainen@live.fi> * Organize all router related files under directory Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>
2025-05-20 05:10:56 +00:00 · 2022-03-11 12:20:00 +02:00
1 parent 772e879b81
commit 38af26efc9
@@ -0,0 +1,13 @@
+/**
+ * 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";
+
+const parseRequestInjectable = getInjectable({
+  id: "parse-http-request",
+  instantiate: () => Subtext.parse,
+});
+
+export default parseRequestInjectable;
@@ -0,0 +1,86 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import type { LensApiResult } from "./router";
+
+export interface LensApiResultContentType {
+  resultMapper: (result: LensApiResult<any>) => ({
+    statusCode: number;
+    content: any;
+    headers: Record<string, string>;
+  });
+}
+
+const resultMapperFor =
+  (contentType: string): LensApiResultContentType["resultMapper"] =>
+    ({ response, error, statusCode= error ? 400 : 200, headers = {}}) => ({
+      statusCode,
+      content: error || response,
+      headers: { ...headers, "Content-Type": contentType },
+    });
+
+export type SupportedFileExtension = "json" | "txt" | "html" | "css" | "gif" | "jpg" | "png" | "svg" | "js" | "woff2" | "ttf";
+
+export const contentTypes: Record<SupportedFileExtension, LensApiResultContentType> = {
+  json: {
+    resultMapper: (result) => {
+      const resultMapper = resultMapperFor("application/json");
+
+      const mappedResult = resultMapper(result);
+
+      const contentIsObject = typeof mappedResult.content === "object";
+      const contentIsBuffer = mappedResult.content instanceof Buffer;
+      const contentShouldBeStringified = contentIsObject && !contentIsBuffer;
+
+      const content = contentShouldBeStringified
+        ? JSON.stringify(mappedResult.content)
+        : mappedResult.content;
+
+      return {
+        ...mappedResult,
+        content,
+      };
+    },
+  },
+
+  txt: {
+    resultMapper: resultMapperFor("text/plain"),
+  },
+
+  html: {
+    resultMapper: resultMapperFor("text/html"),
+  },
+
+  css: {
+    resultMapper: resultMapperFor("text/css"),
+  },
+
+  gif: {
+    resultMapper: resultMapperFor("image/gif"),
+  },
+
+  jpg: {
+    resultMapper: resultMapperFor("image/jpeg"),
+  },
+
+  png: {
+    resultMapper: resultMapperFor("image/png"),
+  },
+
+  svg: {
+    resultMapper: resultMapperFor("image/svg+xml"),
+  },
+
+  js: {
+    resultMapper: resultMapperFor("application/javascript"),
+  },
+
+  woff2: {
+    resultMapper: resultMapperFor("font/woff2"),
+  },
+
+  ttf: {
+    resultMapper: resultMapperFor("font/ttf"),
+  },
+};
@@ -2,23 +2,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 isDevelopmentInjectable from "../../common/vars/is-development.injectable";
-import httpProxy from "http-proxy";
-import { Router } from "../router";
-import routePortForwardInjectable
-  from "../routes/port-forward/route-port-forward/route-port-forward.injectable";
+import { getInjectable, getInjectionToken } from "@ogre-tools/injectable";
+import { Route, Router } from "./router";
+import parseRequestInjectable from "./parse-request.injectable";
+
+export const routeInjectionToken = getInjectionToken<Route<any>>({
+  id: "route-injection-token",
+});

 const routerInjectable = getInjectable({
  id: "router",

  instantiate: (di) => {
-    const isDevelopment = di.inject(isDevelopmentInjectable);
-    const proxy = isDevelopment ? httpProxy.createProxy() : undefined;
+    const routes = di.injectMany(routeInjectionToken);

-    return new Router({
-      routePortForward: di.inject(routePortForwardInjectable),
-      httpProxy: proxy,
+    return new Router(routes, {
+      parseRequest: di.inject(parseRequestInjectable),
    });
  },
 });
@@ -0,0 +1,294 @@
+/**
+ * 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, RouteHandler, Route } 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 mockFs from "mock-fs";
+
+describe("router", () => {
+  let router: Router;
+  let routeHandlerMock: AsyncFnMock<RouteHandler<any>>;
+
+  beforeEach(async () => {
+    routeHandlerMock = asyncFn();
+
+    const di = getDiForUnitTesting({ doGeneralOverrides: true });
+
+    mockFs();
+
+    di.override(parseRequestInjectable, () => () => Promise.resolve({ payload: "some-payload" }));
+
+    await di.runSetups();
+
+    const injectable = getInjectable({
+      id: "some-route",
+
+      instantiate: () => ({
+        method: "get",
+        path: "/some-path",
+        handler: routeHandlerMock,
+      } as Route<any>),
+
+      injectionToken: routeInjectionToken,
+    });
+
+    di.register(injectable);
+
+    router = di.inject(routerInjectable);
+  });
+
+  afterEach(() => {
+    mockFs.restore();
+  });
+
+  describe("when navigating to the route", () => {
+    let actualPromise: Promise<boolean>;
+    let clusterStub: Cluster;
+    let requestStub: Request;
+    let responseStub: any;
+
+    beforeEach(() => {
+      requestStub = new Request({
+        url: "/some-path",
+        method: "get",
+        headers: {
+          "content-type": "application/json",
+        },
+      });
+
+      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([]);
+        });
+      });
+    });
+  });
+});
@@ -0,0 +1,196 @@
+/**
+ * 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 httpProxy from "http-proxy";
+import { toPairs } from "lodash/fp";
+import path from "path";
+import type { Cluster } from "../../common/cluster/cluster";
+import type { LensApiResultContentType } from "./router-content-types";
+import { contentTypes } from "./router-content-types";
+
+// TODO: Import causes side effect, sets value for __static
+import "../../common/vars";
+
+export interface RouterRequestOpts {
+  req: http.IncomingMessage;
+  res: http.ServerResponse;
+  cluster: Cluster;
+  params: RouteParams;
+  url: URL;
+}
+
+export interface RouteParams extends Record<string, string> {
+  path?: string; // *-route
+  namespace?: string;
+  service?: string;
+  account?: string;
+  release?: string;
+  repo?: string;
+  chart?: string;
+}
+
+export interface LensApiRequest<P = any> {
+  path: string;
+  payload: P;
+  params: RouteParams;
+  cluster: Cluster;
+  query: URLSearchParams;
+  raw: {
+    req: http.IncomingMessage;
+    res: http.ServerResponse;
+  };
+}
+
+interface Dependencies {
+  parseRequest: (request: http.IncomingMessage, _: null, options: { parse: boolean; output: string }) => Promise<{ payload: any }>;
+}
+
+export class Router {
+  protected router = new Call.Router();
+  protected static rootPath = path.resolve(__static);
+
+  constructor(routes: Route<unknown>[], private dependencies: Dependencies) {
+    routes.forEach(route => {
+      this.router.add({ method: route.method, path: route.path }, handleRoute(route));
+    });
+  }
+
+  public async route(cluster: Cluster, req: http.IncomingMessage, 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);
+    const routeFound = !matchingRoute.isBoom;
+
+    if (routeFound) {
+      const request = await this.getRequest({ req, res, cluster, url, params: matchingRoute.params });
+
+      await matchingRoute.route(request, res);
+
+      return true;
+    }
+
+    return false;
+  }
+
+  protected async getRequest(opts: RouterRequestOpts): Promise<LensApiRequest> {
+    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,
+    };
+  }
+}
+
+export interface LensApiResult<TResult> {
+  statusCode?: number;
+  response?: TResult;
+  error?: any;
+  contentType?: LensApiResultContentType;
+  headers?: { [name: string]: string };
+  proxy?: httpProxy;
+}
+
+export type RouteHandler<TResponse> = (
+  request: LensApiRequest
+) =>
+  | Promise<LensApiResult<TResponse>>
+  | Promise<void>
+  | LensApiResult<TResponse>
+  | void;
+
+export interface Route<TResponse> {
+  path: string;
+  method: "get" | "post" | "put" | "patch" | "delete";
+  handler: RouteHandler<TResponse>;
+}
+
+const handleRoute = (route: Route<unknown>) => async (request: LensApiRequest, response: http.ServerResponse) => {
+  let result: LensApiResult<any> | void;
+
+  const writeServerResponse = writeServerResponseFor(response);
+
+  try {
+    result = await route.handler(request);
+  } catch(error) {
+    const mappedResult = contentTypes.txt.resultMapper({
+      statusCode: 500,
+      error: error.toString(),
+    });
+
+    writeServerResponse(mappedResult);
+
+    return;
+  }
+
+  if (!result) {
+    const mappedResult = contentTypes.txt.resultMapper({
+      statusCode: 204,
+      response: undefined,
+    });
+
+    writeServerResponse(mappedResult);
+
+    return;
+  }
+
+  if (result.proxy) {
+    return;
+  }
+
+  const contentType = result.contentType || contentTypes.json;
+
+  const mappedResult = contentType.resultMapper(result);
+
+  writeServerResponse(mappedResult);
+};
+
+const writeServerResponseFor =
+  (serverResponse: http.ServerResponse) =>
+    ({
+      statusCode,
+      content,
+      headers,
+    }: {
+    statusCode: number;
+    content: any;
+    headers: { [name: string]: string };
+  }) => {
+      serverResponse.statusCode = statusCode;
+
+      const headerPairs = toPairs<string>(headers);
+
+      headerPairs.forEach(([name, value]) => {
+        serverResponse.setHeader(name, value);
+      });
+
+      if (content instanceof Buffer) {
+        serverResponse.write(content);
+
+        serverResponse.end();
+
+        return;
+      }
+
+      if (content) {
+        serverResponse.end(content);
+      } else {
+        serverResponse.end();
+      }
+    };