memoize middlewares so we don't churn resources excessively

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

plugins/kubernetes-backend/api-report.md

@@ -354,7 +354,7 @@ export type KubernetesObjectTypes =
 export class KubernetesProxy {
   constructor(logger: Logger, clusterSupplier: KubernetesClustersSupplier);
   // (undocumented)
-  proxyRequestHandler: RequestHandler;
+  createRequestHandler(): RequestHandler;
 }
 
 // @alpha

plugins/kubernetes-backend/src/service/KubernetesBuilder.ts

@@ -299,7 +299,7 @@ export class KubernetesBuilder {
       });
     });
 
-    router.use('/proxy', proxy.proxyRequestHandler);
+    router.use('/proxy', proxy.createRequestHandler());
 
     addResourceRoutesToRouter(router, catalogApi, objectsProvider);
 

plugins/kubernetes-backend/src/service/KubernetesProxy.test.ts

@@ -13,8 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import 'buffer';
 
+import 'buffer';
 import { getVoidLogger } from '@backstage/backend-common';
 import { NotFoundError } from '@backstage/errors';
 import { getMockReq, getMockRes } from '@jest-mock/express';
@@ -24,7 +24,6 @@ import request from 'supertest';
 import { rest } from 'msw';
 import { setupServer } from 'msw/node';
 import { setupRequestMockHandlers } from '@backstage/backend-test-utils';
-
 import { ClusterDetails, KubernetesClustersSupplier } from '../types/types';
 import {
   APPLICATION_JSON,
@@ -76,7 +75,7 @@ describe('KubernetesProxy', () => {
     const req = buildMockRequest('test', 'api');
     const { res, next } = getMockRes();
 
-    await expect(proxy.proxyRequestHandler(req, res, next)).rejects.toThrow(
+    await expect(proxy.createRequestHandler()(req, res, next)).rejects.toThrow(
       NotFoundError,
     );
   });
@@ -101,7 +100,7 @@ describe('KubernetesProxy', () => {
         authProvider: 'serviceAccount',
       },
     ] as ClusterDetails[]);
-    const app = express().use('/mountpath', proxy.proxyRequestHandler);
+    const app = express().use('/mountpath', proxy.createRequestHandler());
     const requestPromise = request(app)
       .get('/mountpath/api')
       .set(HEADER_KUBERNETES_CLUSTER, 'cluster1');

plugins/kubernetes-backend/src/service/KubernetesProxy.ts

@@ -23,10 +23,7 @@ import {
 } from '@backstage/errors';
 import { bufferFromFileOrString } from '@kubernetes/client-node';
 import type { Request, RequestHandler } from 'express';
-import {
-  createProxyMiddleware,
-  Options as ProxyMiddlewareOptions,
-} from 'http-proxy-middleware';
+import { createProxyMiddleware } from 'http-proxy-middleware';
 import { Logger } from 'winston';
 import { ClusterDetails, KubernetesClustersSupplier } from '../types/types';
 
@@ -45,79 +42,89 @@ export const HEADER_KUBERNETES_CLUSTER: string = 'X-Kubernetes-Cluster';
  * @alpha
  */
 export class KubernetesProxy {
+  private readonly middlewareForClusterName = new Map<string, RequestHandler>();
+
   constructor(
     private readonly logger: Logger,
     private readonly clusterSupplier: KubernetesClustersSupplier,
   ) {}
 
-  public proxyRequestHandler: RequestHandler = async (req, res, next) => {
-    const requestedCluster = this.getKubernetesRequestedCluster(req);
-
-    const clusterDetails = await this.getClusterDetails(requestedCluster);
-
-    const clusterUrl = new URL(clusterDetails.url);
-    const options: ProxyMiddlewareOptions = {
-      logProvider: () => this.logger,
-      secure: !clusterDetails.skipTLSVerify,
-      target: {
-        protocol: clusterUrl.protocol,
-        host: clusterUrl.hostname,
-        port: clusterUrl.port,
-        ca: bufferFromFileOrString('', clusterDetails.caData)?.toString(),
-      },
-      pathRewrite: { [`^${req.baseUrl}`]: '' },
-      onError: (error: Error) => {
-        const wrappedError = new ForwardedError(
-          `Cluster '${requestedCluster}' request error`,
-          error,
-        );
-
-        this.logger.error(wrappedError);
-
-        const body: ErrorResponseBody = {
-          error: serializeError(wrappedError, {
-            includeStack: process.env.NODE_ENV === 'development',
-          }),
-          request: { method: req.method, url: req.originalUrl },
-          response: { statusCode: 500 },
-        };
-
-        res.status(500).json(body);
-      },
+  public createRequestHandler(): RequestHandler {
+    return async (req, res, next) => {
+      const middleware = await this.getMiddleware(req);
+      middleware(req, res, next);
     };
+  }
 
-    // Probably too risky without permissions protecting this endpoint
-    // if (clusterDetails.serviceAccountToken) {
-    //   options.headers = {
-    //     Authorization: `Bearer ${clusterDetails.serviceAccountToken}`,
-    //   };
-    // }
-    createProxyMiddleware(options)(req, res, next);
-  };
+  // We create one middleware per remote cluster and hold on to them, because
+  // the secure property isn't possible to decide on a per-request basis with a
+  // single middleware instance - and we don't expect it to change over time.
+  private async getMiddleware(originalReq: Request): Promise<RequestHandler> {
+    const originalCluster = await this.getClusterForRequest(originalReq);
+    let middleware = this.middlewareForClusterName.get(originalCluster.name);
+    if (!middleware) {
+      // Probably too risky without permissions protecting this endpoint
+      // if (cluster.serviceAccountToken) {
+      //   options.headers = {
+      //     Authorization: `Bearer ${cluster.serviceAccountToken}`,
+      //   };
+      // }
 
-  private getKubernetesRequestedCluster(req: Request): string {
-    const requestedClusterName = req.header(HEADER_KUBERNETES_CLUSTER);
+      const logger = this.logger.child({ cluster: originalCluster.name });
+      middleware = createProxyMiddleware({
+        logProvider: () => logger,
+        secure: !originalCluster.skipTLSVerify,
+        router: async req => {
+          // Re-evaluate the cluster on each request, in case it has changed
+          const cluster = await this.getClusterForRequest(req);
+          const url = new URL(cluster.url);
+          return {
+            protocol: url.protocol,
+            host: url.hostname,
+            port: url.port,
+            ca: bufferFromFileOrString('', cluster.caData)?.toString(),
+          };
+        },
+        pathRewrite: { [`^${originalReq.baseUrl}`]: '' },
+        onError: (error, req, res) => {
+          const wrappedError = new ForwardedError(
+            `Cluster '${originalCluster.name}' request error`,
+            error,
+          );
 
-    if (!requestedClusterName) {
+          logger.error(wrappedError);
+
+          const body: ErrorResponseBody = {
+            error: serializeError(wrappedError, {
+              includeStack: process.env.NODE_ENV === 'development',
+            }),
+            request: { method: req.method, url: req.originalUrl },
+            response: { statusCode: 500 },
+          };
+
+          res.status(500).json(body);
+        },
+      });
+
+      this.middlewareForClusterName.set(originalCluster.name, middleware);
+    }
+
+    return middleware;
+  }
+
+  private async getClusterForRequest(req: Request): Promise<ClusterDetails> {
+    const clusterName = req.header(HEADER_KUBERNETES_CLUSTER);
+    if (!clusterName) {
       throw new InputError(`Missing '${HEADER_KUBERNETES_CLUSTER}' header.`);
     }
 
-    return requestedClusterName;
-  }
-
-  private async getClusterDetails(
-    requestedCluster: string,
-  ): Promise<ClusterDetails> {
-    const clusters = await this.clusterSupplier.getClusters();
-
-    const clusterDetail = clusters.find(
-      cluster => cluster.name === requestedCluster,
-    );
-
-    if (!clusterDetail) {
-      throw new NotFoundError(`Cluster '${requestedCluster}' not found`);
+    const cluster = await this.clusterSupplier
+      .getClusters()
+      .then(clusters => clusters.find(c => c.name === clusterName));
+    if (!cluster) {
+      throw new NotFoundError(`Cluster '${clusterName}' not found`);
     }
 
-    return clusterDetail;
+    return cluster;
   }
 }