From 8ab897e0181eb4c6e9ca0c7f8624dee0cf78faf6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= <freben@gmail.com>
Date: Mon, 28 Jun 2021 15:54:14 +0200
Subject: [PATCH] Properly return a 404 when an unknown cluster is given
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Fredrik Adelöw <freben@gmail.com>
---
 .changeset/warm-mangos-share.md                  |  5 +++++
 plugins/kafka-backend/package.json               |  1 +
 plugins/kafka-backend/src/service/router.test.ts | 15 ++++++++++++++-
 plugins/kafka-backend/src/service/router.ts      | 14 ++++++++++++--
 4 files changed, 32 insertions(+), 3 deletions(-)
 create mode 100644 .changeset/warm-mangos-share.md

diff --git a/.changeset/warm-mangos-share.md b/.changeset/warm-mangos-share.md
new file mode 100644
index 0000000000..4f54b100d2
--- /dev/null
+++ b/.changeset/warm-mangos-share.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-kafka-backend': patch
+---
+
+Properly return a 404 when an unknown cluster is given
diff --git a/plugins/kafka-backend/package.json b/plugins/kafka-backend/package.json
index 617dd2d3cb..d85b0974d6 100644
--- a/plugins/kafka-backend/package.json
+++ b/plugins/kafka-backend/package.json
@@ -34,6 +34,7 @@
     "@backstage/backend-common": "^0.8.2",
     "@backstage/catalog-model": "^0.8.2",
     "@backstage/config": "^0.1.5",
+    "@backstage/errors": "^0.1.1",
     "@types/express": "^4.17.6",
     "express": "^4.17.1",
     "express-promise-router": "^4.1.0",
diff --git a/plugins/kafka-backend/src/service/router.test.ts b/plugins/kafka-backend/src/service/router.test.ts
index d3f19f720f..4f458fe543 100644
--- a/plugins/kafka-backend/src/service/router.test.ts
+++ b/plugins/kafka-backend/src/service/router.test.ts
@@ -17,7 +17,7 @@
 import request from 'supertest';
 import express from 'express';
 import { makeRouter, ClusterApi } from './router';
-import { getVoidLogger } from '@backstage/backend-common';
+import { errorHandler, getVoidLogger } from '@backstage/backend-common';
 import { KafkaApi } from './KafkaApi';
 import { when } from 'jest-when';
 
@@ -111,6 +111,19 @@ describe('router', () => {
       );
     });
 
+    it('handles unknown cluster errors correctly', async () => {
+      const response = await request(app.use(errorHandler())).get(
+        '/consumers/unknown/hey/offsets',
+      );
+      expect(response.status).toEqual(404);
+      expect(response.body).toMatchObject({
+        error: {
+          message:
+            'Found no configured cluster "unknown", candidates are "dev", "prod"',
+        },
+      });
+    });
+
     it('handles internal error correctly', async () => {
       prodKafkaApi.fetchGroupOffsets.mockRejectedValue(Error('oh no'));
 
diff --git a/plugins/kafka-backend/src/service/router.ts b/plugins/kafka-backend/src/service/router.ts
index 68f3924f89..6c8bce7af3 100644
--- a/plugins/kafka-backend/src/service/router.ts
+++ b/plugins/kafka-backend/src/service/router.ts
@@ -18,6 +18,7 @@ import express from 'express';
 import Router from 'express-promise-router';
 import { Logger } from 'winston';
 import { Config } from '@backstage/config';
+import { NotFoundError } from '@backstage/errors';
 import { KafkaApi, KafkaJsApiImpl } from './KafkaApi';
 import _ from 'lodash';
 import { getClusterDetails } from '../config/ClusterReader';
@@ -45,12 +46,20 @@ export const makeRouter = (
     const clusterId = req.params.clusterId;
     const consumerId = req.params.consumerId;
 
+    const kafkaApi = kafkaApiByClusterName[clusterId];
+    if (!kafkaApi) {
+      const candidates = Object.keys(kafkaApiByClusterName)
+        .map(n => `"${n}"`)
+        .join(', ');
+      throw new NotFoundError(
+        `Found no configured cluster "${clusterId}", candidates are ${candidates}`,
+      );
+    }
+
     logger.info(
       `Fetch consumer group ${consumerId} offsets from cluster ${clusterId}`,
     );
 
-    const kafkaApi = kafkaApiByClusterName[clusterId];
-
     const groupOffsets = await kafkaApi.api.fetchGroupOffsets(consumerId);
 
     const groupWithTopicOffsets = await Promise.all(
@@ -68,6 +77,7 @@ export const makeRouter = (
         }));
       }),
     );
+
     res.json({ consumerId, offsets: groupWithTopicOffsets.flat() });
   });